Public Access
DEVX-127: fix: fall back to CI token when reviewer self-approval is rejected
This commit was merged in pull request #192.
This commit is contained in:
@@ -157,6 +157,7 @@ jobs:
|
|||||||
- name: Post approval review
|
- name: Post approval review
|
||||||
env:
|
env:
|
||||||
REVIEWER_GITEA_API_TOKEN: ${{ secrets.REVIEWER_GITEA_API_TOKEN }}
|
REVIEWER_GITEA_API_TOKEN: ${{ secrets.REVIEWER_GITEA_API_TOKEN }}
|
||||||
|
CI_GITEA_API_TOKEN: ${{ secrets.CI_GITEA_API_TOKEN }}
|
||||||
PR_NUMBER: ${{ github.event.number }}
|
PR_NUMBER: ${{ github.event.number }}
|
||||||
REPOSITORY: ${{ github.repository }}
|
REPOSITORY: ${{ github.repository }}
|
||||||
run: |
|
run: |
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ Usage:
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
import re
|
import re
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@@ -548,8 +549,14 @@ def _post_manual_review(
|
|||||||
checklist_confirmed: bool,
|
checklist_confirmed: bool,
|
||||||
checklist_categories: str | None,
|
checklist_categories: str | None,
|
||||||
dry_run: bool,
|
dry_run: bool,
|
||||||
|
owner: str | None = None,
|
||||||
|
repo_name: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Post a manual review with validation for APPROVE events."""
|
"""Post a manual review with validation for APPROVE events.
|
||||||
|
|
||||||
|
When self-approval is rejected (reviewer token belongs to PR author),
|
||||||
|
falls back to the CI token (different user) if available.
|
||||||
|
"""
|
||||||
if not body or len(body) < 50:
|
if not body or len(body) < 50:
|
||||||
raise click.ClickException(_("Review body must be at least 50 characters."))
|
raise click.ClickException(_("Review body must be at least 50 characters."))
|
||||||
|
|
||||||
@@ -585,8 +592,20 @@ def _post_manual_review(
|
|||||||
review = client.create_review(pr_number, event=event, body=body)
|
review = client.create_review(pr_number, event=event, body=body)
|
||||||
except APIError as e:
|
except APIError as e:
|
||||||
if "approve" in e.message.lower() or "422" in str(e.status):
|
if "approve" in e.message.lower() or "422" in str(e.status):
|
||||||
click.echo(_("Note: Self-approval not allowed. Posting COMMENT instead."))
|
# Self-approval not allowed (reviewer token belongs to PR author).
|
||||||
review = client.create_review(pr_number, event="COMMENT", body=body)
|
# Fall back to CI token (different user) if available.
|
||||||
|
ci_token = os.environ.get("CI_GITEA_API_TOKEN", "").strip()
|
||||||
|
if ci_token and owner and repo_name:
|
||||||
|
click.echo(_("Note: Self-approval not allowed with reviewer token. Retrying with CI token."))
|
||||||
|
ci_client = GiteaClient(GITEA_API_URL, ci_token, owner, repo_name)
|
||||||
|
try:
|
||||||
|
review = ci_client.create_review(pr_number, event=event, body=body)
|
||||||
|
except APIError:
|
||||||
|
click.echo(_("Note: CI token also cannot approve. Posting COMMENT instead."))
|
||||||
|
review = client.create_review(pr_number, event="COMMENT", body=body)
|
||||||
|
else:
|
||||||
|
click.echo(_("Note: Self-approval not allowed. Posting COMMENT instead."))
|
||||||
|
review = client.create_review(pr_number, event="COMMENT", body=body)
|
||||||
else:
|
else:
|
||||||
raise
|
raise
|
||||||
review_id = review.get("id", "?")
|
review_id = review.get("id", "?")
|
||||||
@@ -645,7 +664,17 @@ def main(
|
|||||||
client = GiteaClient(GITEA_API_URL, token, owner, repo_name)
|
client = GiteaClient(GITEA_API_URL, token, owner, repo_name)
|
||||||
|
|
||||||
if event is not None:
|
if event is not None:
|
||||||
_post_manual_review(client, pr_number, event.upper(), body, checklist_confirmed, checklist_categories, dry_run)
|
_post_manual_review(
|
||||||
|
client,
|
||||||
|
pr_number,
|
||||||
|
event.upper(),
|
||||||
|
body,
|
||||||
|
checklist_confirmed,
|
||||||
|
checklist_categories,
|
||||||
|
dry_run,
|
||||||
|
owner=owner,
|
||||||
|
repo_name=repo_name,
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
result = run_review(client, pr_number)
|
result = run_review(client, pr_number)
|
||||||
|
|||||||
@@ -2104,12 +2104,28 @@
|
|||||||
"zh": "No workflow runs found for SHA {sha}."
|
"zh": "No workflow runs found for SHA {sha}."
|
||||||
},
|
},
|
||||||
"Note: Self-approval not allowed. Posting COMMENT instead.": {
|
"Note: Self-approval not allowed. Posting COMMENT instead.": {
|
||||||
"bg": "Note: Self-approval not allowed. Posting COMMENT instead.",
|
"bg": "Забележка: Само-одобрението не е разрешено. Публикуване на COMMENT вместо това.",
|
||||||
"de": "Note: Self-approval not allowed. Posting COMMENT instead.",
|
"de": "Hinweis: Selbstgenehmigung nicht erlaubt. COMMENT wird stattdessen gesendet.",
|
||||||
"en": "Note: Self-approval not allowed. Posting COMMENT instead.",
|
"en": "Note: Self-approval not allowed. Posting COMMENT instead.",
|
||||||
"pl": "Uwaga: Samo-zatwierdzenie niedozwolone. Publikowanie COMMENT zamiast tego.",
|
"pl": "Uwaga: Samo-zatwierdzenie niedozwolone. Publikowanie COMMENT zamiast tego.",
|
||||||
"ru": "Note: Self-approval not allowed. Posting COMMENT instead.",
|
"ru": "Примечание: Самоодобрение не разрешено. Публикация COMMENT вместо этого.",
|
||||||
"zh": "Note: Self-approval not allowed. Posting COMMENT instead."
|
"zh": "注意:不允许自我批准。改为发布 COMMENT。"
|
||||||
|
},
|
||||||
|
"Note: Self-approval not allowed with reviewer token. Retrying with CI token.": {
|
||||||
|
"bg": "Забележка: Само-одобрението не е разрешено с тоукън на рецензента. Повторен опит с CI тоукън.",
|
||||||
|
"de": "Hinweis: Selbstgenehmigung mit Reviewer-Token nicht erlaubt. Wiederholung mit CI-Token.",
|
||||||
|
"en": "Note: Self-approval not allowed with reviewer token. Retrying with CI token.",
|
||||||
|
"pl": "Uwaga: Samo-zatwierdzenie niedozwolone tokenem recenzenta. Ponawianie tokenem CI.",
|
||||||
|
"ru": "Примечание: Самоодобрение токеном ревьюера не разрешено. Повторная попытка с CI токеном.",
|
||||||
|
"zh": "注意:不允许使用审阅者令牌进行自我批准。正在使用 CI 令牌重试。"
|
||||||
|
},
|
||||||
|
"Note: CI token also cannot approve. Posting COMMENT instead.": {
|
||||||
|
"bg": "Забележка: CI тоукънът също не може да одобри. Публикуване на COMMENT вместо това.",
|
||||||
|
"de": "Hinweis: CI-Token kann ebenfalls nicht genehmigen. COMMENT wird stattdessen gesendet.",
|
||||||
|
"en": "Note: CI token also cannot approve. Posting COMMENT instead.",
|
||||||
|
"pl": "Uwaga: Token CI również nie może zatwierdzić. Publikowanie COMMENT zamiast tego.",
|
||||||
|
"ru": "Примечание: CI токен также не может одобрить. Публикация COMMENT вместо этого.",
|
||||||
|
"zh": "注意:CI 令牌也无法批准。改为发布 COMMENT。"
|
||||||
},
|
},
|
||||||
"Nothing to push.": {
|
"Nothing to push.": {
|
||||||
"bg": "Nothing to push.",
|
"bg": "Nothing to push.",
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
from click.testing import CliRunner
|
from click.testing import CliRunner
|
||||||
|
|
||||||
from devx.ci.pr_review import (
|
from devx.ci.pr_review import (
|
||||||
@@ -902,7 +903,12 @@ class TestManualReview:
|
|||||||
mock_client_class.return_value.create_review.assert_not_called()
|
mock_client_class.return_value.create_review.assert_not_called()
|
||||||
|
|
||||||
@patch("devx.ci.pr_review.GiteaClient")
|
@patch("devx.ci.pr_review.GiteaClient")
|
||||||
def test_manual_review_self_approval_fallback(self, mock_client_class: MagicMock) -> None:
|
def test_manual_review_self_approval_fallback_to_comment(
|
||||||
|
self, mock_client_class: MagicMock, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
"""Self-approval with no CI token available → fall back to COMMENT."""
|
||||||
|
monkeypatch.delenv("CI_GITEA_API_TOKEN", raising=False)
|
||||||
|
monkeypatch.delenv("CI_GITEA_TOKEN", raising=False)
|
||||||
client = mock_client_class.return_value
|
client = mock_client_class.return_value
|
||||||
client.create_review.side_effect = [
|
client.create_review.side_effect = [
|
||||||
APIError(422, "approve your own pull is not allowed"),
|
APIError(422, "approve your own pull is not allowed"),
|
||||||
@@ -922,10 +928,77 @@ class TestManualReview:
|
|||||||
"--checklist-categories",
|
"--checklist-categories",
|
||||||
"1,2,3,4,5,6,7,8",
|
"1,2,3,4,5,6,7,8",
|
||||||
],
|
],
|
||||||
env={"CI_GITEA_TOKEN": "fake"},
|
env={"REVIEWER_GITEA_API_TOKEN": "fake-reviewer"},
|
||||||
)
|
)
|
||||||
assert result.exit_code == 0
|
assert result.exit_code == 0
|
||||||
assert "Review #202" in result.output
|
assert "Review #202" in result.output
|
||||||
|
# Without CI_GITEA_API_TOKEN, the fallback is COMMENT
|
||||||
|
assert "Self-approval not allowed. Posting COMMENT instead." in result.output
|
||||||
|
assert client.create_review.call_count == 2
|
||||||
|
assert client.create_review.call_args_list[1].kwargs.get("event") == "COMMENT"
|
||||||
|
|
||||||
|
@patch("devx.ci.pr_review.GiteaClient")
|
||||||
|
def test_manual_review_self_approval_falls_back_to_ci_token(self, mock_client_class: MagicMock) -> None:
|
||||||
|
"""Self-approval with CI token available → retry APPROVE with CI token (different user)."""
|
||||||
|
client = mock_client_class.return_value
|
||||||
|
client.create_review.side_effect = [
|
||||||
|
APIError(422, "approve your own pull is not allowed"),
|
||||||
|
{"id": 303},
|
||||||
|
]
|
||||||
|
runner = CliRunner()
|
||||||
|
result = runner.invoke(
|
||||||
|
main,
|
||||||
|
[
|
||||||
|
"42",
|
||||||
|
"oblachno-oss/devx",
|
||||||
|
"--event",
|
||||||
|
"APPROVE",
|
||||||
|
"--body",
|
||||||
|
"x" * 60,
|
||||||
|
"--checklist-confirmed",
|
||||||
|
"--checklist-categories",
|
||||||
|
"1,2,3,4,5,6,7,8",
|
||||||
|
],
|
||||||
|
env={"REVIEWER_GITEA_API_TOKEN": "fake-reviewer", "CI_GITEA_API_TOKEN": "fake-ci"},
|
||||||
|
)
|
||||||
|
assert result.exit_code == 0
|
||||||
|
assert "Review #303" in result.output
|
||||||
|
assert "Retrying with CI token" in result.output
|
||||||
|
# Second call should still be APPROVE (CI token retry)
|
||||||
|
assert client.create_review.call_count == 2
|
||||||
|
assert client.create_review.call_args_list[1].kwargs.get("event") == "APPROVE"
|
||||||
|
|
||||||
|
@patch("devx.ci.pr_review.GiteaClient")
|
||||||
|
def test_manual_review_ci_token_also_fails_falls_back_to_comment(self, mock_client_class: MagicMock) -> None:
|
||||||
|
"""Self-approval + CI token retry also fails → fall back to COMMENT."""
|
||||||
|
client = mock_client_class.return_value
|
||||||
|
client.create_review.side_effect = [
|
||||||
|
APIError(422, "approve your own pull is not allowed"),
|
||||||
|
APIError(422, "approve your own pull is not allowed"),
|
||||||
|
{"id": 404},
|
||||||
|
]
|
||||||
|
runner = CliRunner()
|
||||||
|
result = runner.invoke(
|
||||||
|
main,
|
||||||
|
[
|
||||||
|
"42",
|
||||||
|
"oblachno-oss/devx",
|
||||||
|
"--event",
|
||||||
|
"APPROVE",
|
||||||
|
"--body",
|
||||||
|
"x" * 60,
|
||||||
|
"--checklist-confirmed",
|
||||||
|
"--checklist-categories",
|
||||||
|
"1,2,3,4,5,6,7,8",
|
||||||
|
],
|
||||||
|
env={"REVIEWER_GITEA_API_TOKEN": "fake-reviewer", "CI_GITEA_API_TOKEN": "fake-ci"},
|
||||||
|
)
|
||||||
|
assert result.exit_code == 0
|
||||||
|
assert "Review #404" in result.output
|
||||||
|
assert "CI token also cannot approve" in result.output
|
||||||
|
# Third call should be COMMENT (final fallback)
|
||||||
|
assert client.create_review.call_count == 3
|
||||||
|
assert client.create_review.call_args_list[2].kwargs.get("event") == "COMMENT"
|
||||||
|
|
||||||
@patch("devx.ci.pr_review.GiteaClient")
|
@patch("devx.ci.pr_review.GiteaClient")
|
||||||
def test_manual_review_other_error_re_raises(self, mock_client_class: MagicMock) -> None:
|
def test_manual_review_other_error_re_raises(self, mock_client_class: MagicMock) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user