feat(git): post_pr_review — post one change-request to a PR

GitService.post_pr_review posts a single review via POST /pulls/{n}/reviews
(REQUEST_CHANGES by default; APPROVE/COMMENT supported) — the first /reviews
call in the codebase. Resolves owner/repo/token from the project slug,
authenticates as the PAT owner (Bearer), and raises GitError on any token or
GitHub failure so the calling side-effect can surface it. This is the capability
the pr_reviewer's post_pr_review verb invokes after its DB commit. httpx fully
mocked in tests (request shape, auth, error paths).
This commit is contained in:
Renn F
2026-06-16 10:41:09 +02:00
parent 5902c0fe38
commit c69900ee9c
2 changed files with 146 additions and 0 deletions
+52
View File
@@ -1650,6 +1650,58 @@ class GitService(BaseService):
{"owner": owner, "repo": repo, "pr": pr_number},
)
async def post_pr_review(
self,
project_slug: str,
pr_number: int,
body: str,
*,
event: str = "REQUEST_CHANGES",
) -> dict[str, Any]:
"""Post ONE review to a PR — ``POST /pulls/{n}/reviews``.
``event`` is ``REQUEST_CHANGES`` (default), ``APPROVE``, or ``COMMENT``.
Authenticates as the project's PAT owner (the bot account); the agent /
role that authored the review is named in ``body``, not the GitHub
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).
"""
details = {"project": project_slug, "pr": pr_number}
project = await get_project_service(self.session).get_by_slug(project_slug)
if project is None or not project.git_url:
raise GitError(f"unknown project for PR review: {project_slug!r}", details)
owner, repo = self._parse_git_url(project.git_url)
git_token = await self._token_for_project(project_slug)
if not git_token:
raise GitError(f"no git token for project {project_slug!r}", details)
api_base = settings.github_api_base_url.rstrip("/")
try:
async with httpx.AsyncClient(timeout=_default_git_timeout()) as client:
resp = await client.post(
f"{api_base}/repos/{owner}/{repo}/pulls/{pr_number}/reviews",
headers={
"Authorization": f"Bearer {git_token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
},
json={"body": body, "event": event},
)
except httpx.HTTPError as e:
raise GitError(
f"GitHub API error while posting review to PR #{pr_number}: {e}",
details,
) from e
if resp.status_code == _HTTP_NOT_FOUND:
raise GitError(f"PR not found: #{pr_number} on {owner}/{repo}", details)
if not resp.is_success:
raise GitError(
f"GitHub API refused PR review ({resp.status_code}): "
f"{resp.text[:200]}",
details,
)
return cast("dict[str, Any]", resp.json())
async def update_pr_for_task(
self,
task_id: UUID,
@@ -0,0 +1,94 @@
"""GitService.post_pr_review — posts ONE review to a PR (httpx fully mocked).
Verifies the request shape (the first ``/pulls/{n}/reviews`` call in the
codebase), Bearer auth, and that GitHub/token failures raise ``GitError`` so the
calling side-effect can surface them.
"""
from __future__ import annotations
from typing import Any
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from roboco.services.git import GitError, GitService
_PR = 42
def _service() -> GitService:
session = MagicMock()
session.execute = AsyncMock()
return GitService(session)
def _bind(svc: GitService, name: str, value: object) -> None:
object.__setattr__(svc, name, value)
def _resp(
*, status_code: int, json_payload: dict[str, Any] | None = None, text: str = ""
) -> MagicMock:
resp = MagicMock()
resp.status_code = status_code
resp.is_success = 200 <= status_code < 300 # noqa: PLR2004
resp.json.return_value = json_payload or {}
resp.text = text
return resp
def _client(post_resp: MagicMock) -> MagicMock:
client = MagicMock()
client.__aenter__ = AsyncMock(return_value=client)
client.__aexit__ = AsyncMock(return_value=False)
client.post = AsyncMock(return_value=post_resp)
return client
def _patch_project() -> Any:
fake = MagicMock()
fake.get_by_slug = AsyncMock(
return_value=MagicMock(git_url="https://github.com/acme/repo.git")
)
return patch("roboco.services.git.get_project_service", return_value=fake)
@pytest.mark.asyncio
async def test_post_pr_review_posts_request_changes() -> None:
svc = _service()
_bind(svc, "_token_for_project", AsyncMock(return_value="tok"))
client = _client(
_resp(status_code=200, json_payload={"id": 1, "state": "CHANGES_REQUESTED"})
)
with (
_patch_project(),
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
):
out = await svc.post_pr_review("roboco", _PR, "Please fix X.")
client.post.assert_awaited_once()
call = client.post.await_args
assert f"/pulls/{_PR}/reviews" in call.args[0]
assert call.kwargs["json"] == {"body": "Please fix X.", "event": "REQUEST_CHANGES"}
assert call.kwargs["headers"]["Authorization"] == "Bearer tok"
assert out["state"] == "CHANGES_REQUESTED"
@pytest.mark.asyncio
async def test_post_pr_review_raises_on_missing_token() -> None:
svc = _service()
_bind(svc, "_token_for_project", AsyncMock(return_value=None))
with _patch_project(), pytest.raises(GitError):
await svc.post_pr_review("roboco", _PR, "body")
@pytest.mark.asyncio
async def test_post_pr_review_raises_on_github_error() -> None:
svc = _service()
_bind(svc, "_token_for_project", AsyncMock(return_value="tok"))
client = _client(_resp(status_code=422, text="Unprocessable"))
with (
_patch_project(),
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
pytest.raises(GitError),
):
await svc.post_pr_review("roboco", _PR, "body")