mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(pr-review): post a COMMENT review when GitHub forbids self-review
A pr-reviewer review of an org-authored PR never reached GitHub. The agent side ran correctly (claim → read-only diff → review → post_pr_review → completed + CEO notify), but the GitHub publish 422'd with "Can not request changes on your own pull request": the PR was authored by the same account that owns the project PAT. post_pr_review posts best-effort after the DB transition, so the failure was logged and swallowed — the task completed and the CEO was notified "reviewed" while the PR showed no review. GitHub forbids APPROVE / REQUEST_CHANGES on your own PR but DOES allow a plain COMMENT review. The org's internal PRs (and any PR the PAT owner opened) hit this. Retry once as a COMMENT review on the self-review 422 so the review actually lands; the verdict is already stated in the body. The external/fork-PR path (different author) is unchanged — REQUEST_CHANGES succeeds there and the fallback never fires. Tests: self-review 422 downgrades to COMMENT and returns the COMMENT result; a failing COMMENT retry still surfaces GitError with no infinite loop; the existing non-self 422 still raises.
This commit is contained in:
@@ -2005,6 +2005,13 @@ class GitService(BaseService):
|
|||||||
identity. Raises ``GitError`` on any GitHub failure so the calling
|
identity. Raises ``GitError`` on any GitHub failure so the calling
|
||||||
side-effect can surface it (and stays idempotent — it runs once, after
|
side-effect can surface it (and stays idempotent — it runs once, after
|
||||||
the DB commit).
|
the DB commit).
|
||||||
|
|
||||||
|
Self-review fallback: GitHub forbids ``APPROVE`` / ``REQUEST_CHANGES`` on
|
||||||
|
a PR authored by the token's own account (422 "...on your own pull
|
||||||
|
request"). The org's internal PRs — and any PR the PAT owner opened —
|
||||||
|
hit this, so the review would otherwise never land. A plain ``COMMENT``
|
||||||
|
review IS allowed on your own PR, so this retries once as ``COMMENT``
|
||||||
|
(the verdict is already stated in ``body``).
|
||||||
"""
|
"""
|
||||||
details = {"project": project_slug, "pr": pr_number}
|
details = {"project": project_slug, "pr": pr_number}
|
||||||
project = await get_project_service(self.session).get_by_slug(project_slug)
|
project = await get_project_service(self.session).get_by_slug(project_slug)
|
||||||
@@ -2033,6 +2040,22 @@ class GitService(BaseService):
|
|||||||
) from e
|
) from e
|
||||||
if resp.status_code == _HTTP_NOT_FOUND:
|
if resp.status_code == _HTTP_NOT_FOUND:
|
||||||
raise GitError(f"PR not found: #{pr_number} on {owner}/{repo}", details)
|
raise GitError(f"PR not found: #{pr_number} on {owner}/{repo}", details)
|
||||||
|
if (
|
||||||
|
resp.status_code == _GH_UNPROCESSABLE
|
||||||
|
and event != "COMMENT"
|
||||||
|
and "own pull request" in resp.text.lower()
|
||||||
|
):
|
||||||
|
# Self-review: downgrade to a COMMENT review (allowed on your own PR)
|
||||||
|
# so the review actually posts instead of being silently dropped.
|
||||||
|
self.log.warning(
|
||||||
|
"post_pr_review: self-review forbidden, retrying as COMMENT",
|
||||||
|
project=project_slug,
|
||||||
|
pr=pr_number,
|
||||||
|
requested_event=event,
|
||||||
|
)
|
||||||
|
return await self.post_pr_review(
|
||||||
|
project_slug, pr_number, body, event="COMMENT"
|
||||||
|
)
|
||||||
if not resp.is_success:
|
if not resp.is_success:
|
||||||
raise GitError(
|
raise GitError(
|
||||||
f"GitHub API refused PR review ({resp.status_code}): {resp.text[:200]}",
|
f"GitHub API refused PR review ({resp.status_code}): {resp.text[:200]}",
|
||||||
|
|||||||
@@ -92,3 +92,58 @@ async def test_post_pr_review_raises_on_github_error() -> None:
|
|||||||
pytest.raises(GitError),
|
pytest.raises(GitError),
|
||||||
):
|
):
|
||||||
await svc.post_pr_review("roboco", _PR, "body")
|
await svc.post_pr_review("roboco", _PR, "body")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_post_pr_review_self_review_falls_back_to_comment() -> None:
|
||||||
|
"""A 422 'own pull request' downgrades REQUEST_CHANGES to a COMMENT review."""
|
||||||
|
svc = _service()
|
||||||
|
_bind(svc, "_token_for_project", AsyncMock(return_value="tok"))
|
||||||
|
client = MagicMock()
|
||||||
|
client.__aenter__ = AsyncMock(return_value=client)
|
||||||
|
client.__aexit__ = AsyncMock(return_value=False)
|
||||||
|
# First POST (REQUEST_CHANGES) 422s as a self-review; the COMMENT retry wins.
|
||||||
|
client.post = AsyncMock(
|
||||||
|
side_effect=[
|
||||||
|
_resp(
|
||||||
|
status_code=422,
|
||||||
|
text='{"errors":["Review Can not request changes on your own '
|
||||||
|
'pull request"]}',
|
||||||
|
),
|
||||||
|
_resp(status_code=200, json_payload={"id": 9, "state": "COMMENTED"}),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
with (
|
||||||
|
_patch_project(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
):
|
||||||
|
out = await svc.post_pr_review("roboco", _PR, "Please fix X.")
|
||||||
|
assert client.post.await_count == 2 # noqa: PLR2004
|
||||||
|
first, second = client.post.await_args_list
|
||||||
|
assert first.kwargs["json"]["event"] == "REQUEST_CHANGES"
|
||||||
|
assert second.kwargs["json"]["event"] == "COMMENT"
|
||||||
|
assert second.kwargs["json"]["body"] == "Please fix X."
|
||||||
|
assert out["state"] == "COMMENTED"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_post_pr_review_self_review_comment_failure_raises() -> None:
|
||||||
|
"""If the COMMENT retry also fails, the error surfaces (no infinite loop)."""
|
||||||
|
svc = _service()
|
||||||
|
_bind(svc, "_token_for_project", AsyncMock(return_value="tok"))
|
||||||
|
client = MagicMock()
|
||||||
|
client.__aenter__ = AsyncMock(return_value=client)
|
||||||
|
client.__aexit__ = AsyncMock(return_value=False)
|
||||||
|
client.post = AsyncMock(
|
||||||
|
side_effect=[
|
||||||
|
_resp(status_code=422, text='["...on your own pull request"]'),
|
||||||
|
_resp(status_code=500, text="boom"),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
with (
|
||||||
|
_patch_project(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
pytest.raises(GitError),
|
||||||
|
):
|
||||||
|
await svc.post_pr_review("roboco", _PR, "body")
|
||||||
|
assert client.post.await_count == 2 # noqa: PLR2004
|
||||||
|
|||||||
Reference in New Issue
Block a user