From 6fd4dc592b92a0041ad6fc47173b0c43a67f4067 Mon Sep 17 00:00:00 2001 From: Renn F Date: Thu, 18 Jun 2026 22:56:33 +0200 Subject: [PATCH] fix(pr-review): post a COMMENT review when GitHub forbids self-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- roboco/services/git.py | 23 ++++++++ .../unit/services/test_git_post_pr_review.py | 55 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/roboco/services/git.py b/roboco/services/git.py index 895b2c97..bacf5cdc 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -2005,6 +2005,13 @@ class GitService(BaseService): identity. Raises ``GitError`` on any GitHub failure so the calling side-effect can surface it (and stays idempotent — it runs once, after 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} project = await get_project_service(self.session).get_by_slug(project_slug) @@ -2033,6 +2040,22 @@ class GitService(BaseService): ) from e if resp.status_code == _HTTP_NOT_FOUND: 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: raise GitError( f"GitHub API refused PR review ({resp.status_code}): {resp.text[:200]}", diff --git a/tests/unit/services/test_git_post_pr_review.py b/tests/unit/services/test_git_post_pr_review.py index 0e2b5e4d..f1c854aa 100644 --- a/tests/unit/services/test_git_post_pr_review.py +++ b/tests/unit/services/test_git_post_pr_review.py @@ -92,3 +92,58 @@ async def test_post_pr_review_raises_on_github_error() -> None: pytest.raises(GitError), ): 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