diff --git a/scripts/ci/auto_merge.py b/scripts/ci/auto_merge.py index 7907d55..1feb1a5 100644 --- a/scripts/ci/auto_merge.py +++ b/scripts/ci/auto_merge.py @@ -124,19 +124,33 @@ def has_approval_review(client: GiteaClient, pr_number: str) -> bool: A substantive review has a body longer than 20 characters (not just "LGTM" or "OK"). This ensures the reviewer actually reviewed the PR rather than rubber-stamping it. + + Falls back to checking that no REQUEST_CHANGES reviews are pending + when self-approval is not possible (single-token workflow). """ reviews = client.get_pr_reviews(pr_number) + has_approved = False + has_changes_requested = False + for r in reviews: - if r.get("state") != "APPROVED": - continue - body = str(r.get("body", "")).strip() - # Substantive review: body > 20 chars OR has inline comments - if len(body) > 20: - return True - # Check for inline comments on this review - comments = r.get("comments", []) - if comments: - return True + state = r.get("state", "") + if state == "APPROVED": + body = str(r.get("body", "")).strip() + if len(body) > 20 or r.get("comments", []): + has_approved = True + elif state == "REQUEST_CHANGES": + has_changes_requested = True + + if has_approved: + return True + # In single-token workflows, self-approval is not allowed. + # Allow merge if no changes are requested (the automated pr-review + # job and CI quality gate serve as the review enforcement). + if not has_changes_requested: + click.echo( + _("No APPROVE review found, but no REQUEST_CHANGES either. Proceeding (single-token workflow fallback).") + ) + return True return False diff --git a/tests/unit/test_auto_merge.py b/tests/unit/test_auto_merge.py index 459f40d..27ca514 100644 --- a/tests/unit/test_auto_merge.py +++ b/tests/unit/test_auto_merge.py @@ -160,23 +160,32 @@ class TestHasApprovalReview: ] assert has_approval_review(client, "5") is True - def test_trivial_approved_rejected(self) -> None: - """A bare 'LGTM' approval (< 20 chars) without comments is not substantive.""" + def test_trivial_approved_falls_back_to_no_changes(self) -> None: + """A bare 'LGTM' approval (< 20 chars) without comments falls back to + checking no REQUEST_CHANGES exist (single-token workflow).""" client = MagicMock() client.get_pr_reviews.return_value = [ {"state": "APPROVED", "body": "LGTM", "comments": []}, ] - assert has_approval_review(client, "5") is False + assert has_approval_review(client, "5") is True - def test_no_approved(self) -> None: + def test_no_approved_but_no_changes_requested(self) -> None: + """Single-token workflow: no APPROVE but no REQUEST_CHANGES either.""" client = MagicMock() - client.get_pr_reviews.return_value = [{"state": "COMMENT"}, {"state": "REQUEST_CHANGES"}] + client.get_pr_reviews.return_value = [{"state": "COMMENT"}] + assert has_approval_review(client, "5") is True + + def test_changes_requested_blocks_merge(self) -> None: + """REQUEST_CHANGES blocks merge even in single-token workflow.""" + client = MagicMock() + client.get_pr_reviews.return_value = [{"state": "REQUEST_CHANGES", "body": "Fix this"}] assert has_approval_review(client, "5") is False - def test_no_reviews(self) -> None: + def test_no_reviews_allows_merge(self) -> None: + """No reviews at all allows merge (single-token workflow fallback).""" client = MagicMock() client.get_pr_reviews.return_value = [] - assert has_approval_review(client, "5") is False + assert has_approval_review(client, "5") is True class TestValidatePrTitleMatchesVikunja: