diff --git a/python/robomp/src/github_backend.py b/python/robomp/src/github_backend.py index 38e78c19b..dd6136b70 100644 --- a/python/robomp/src/github_backend.py +++ b/python/robomp/src/github_backend.py @@ -100,6 +100,7 @@ class GitHubBackend(Protocol): body: str, event: str, comments: list[Mapping[str, Any]], + commit_id: str | None = None, ) -> PullRequestReviewInfo: ... async def add_assignees(self, repo: str, number: int, assignees: list[str]) -> None: ... diff --git a/python/robomp/src/github_client.py b/python/robomp/src/github_client.py index 2e9c76646..f65002067 100644 --- a/python/robomp/src/github_client.py +++ b/python/robomp/src/github_client.py @@ -69,6 +69,7 @@ class PullRequestInfo: head_repo: str = "" title: str = "" body: str = "" + head_sha: str = "" @dataclass(slots=True, frozen=True) @@ -77,6 +78,7 @@ class PullRequestFileInfo: status: str additions: int deletions: int + patch: str = "" @dataclass(slots=True, frozen=True) @@ -179,7 +181,13 @@ def _parse_retry_after(resp: httpx.Response) -> float | None: class GitHubClient: """Async + sync facades over a small slice of the GitHub REST API.""" - def __init__(self, token: str, *, transport: httpx.BaseTransport | None = None) -> None: + def __init__( + self, + token: str, + *, + transport: httpx.BaseTransport | None = None, + platform: str = "github", + ) -> None: self._token = token self._headers = { "Authorization": f"Bearer {token}", @@ -188,6 +196,7 @@ class GitHubClient: "User-Agent": "robomp/0.1", } self._transport = transport + self._platform = platform def _client(self) -> httpx.Client: return httpx.Client( @@ -581,6 +590,26 @@ class GitHubClient: f"/repos/{repo}/issues/{number}/labels/{encoded}", ) + def _review_comments_payload(self, comments: list[Mapping[str, Any]]) -> list[dict[str, Any]]: + """Adapt canonical host-tool comment shape to the wire schema for this platform. + + GitHub keeps line/side/start_line/start_side; Forgejo only reads + path/body/new_position (+old_position), so github-only keys are dropped + and `line` is mapped to `new_position` for RIGHT-side comments or + `old_position` for LEFT-side (removed-line) comments. + """ + if self._platform != "forgejo": + return [dict(c) for c in comments] + payload: list[dict[str, Any]] = [] + for c in comments: + entry: dict[str, Any] = {"path": c["path"], "body": c["body"]} + if str(c.get("side", "RIGHT")).upper() == "LEFT": + entry["old_position"] = c["line"] + else: + entry["new_position"] = c["line"] + payload.append(entry) + return payload + async def submit_pr_review( self, *, @@ -589,12 +618,12 @@ class GitHubClient: body: str, event: str, comments: list[Mapping[str, Any]], + commit_id: str | None = None, ) -> PullRequestReviewInfo: - data = await self.request( - "POST", - f"/repos/{repo}/pulls/{pr_number}/reviews", - json={"body": body, "event": event, "comments": comments}, - ) + payload: dict[str, Any] = {"body": body, "event": event, "comments": self._review_comments_payload(comments)} + if commit_id: + payload["commit_id"] = commit_id + data = await self.request("POST", f"/repos/{repo}/pulls/{pr_number}/reviews", json=payload) return _pr_review_from_payload(data) async def add_assignees(self, repo: str, number: int, assignees: list[str]) -> None: @@ -750,6 +779,7 @@ def _pr_file_from_payload(data: Mapping[str, Any]) -> PullRequestFileInfo: status=str(data.get("status") or ""), additions=int(data.get("additions") or 0), deletions=int(data.get("deletions") or 0), + patch=str(data.get("patch") or ""), ) @@ -769,6 +799,7 @@ def _pr_from_payload(repo: str, data: Mapping[str, Any]) -> PullRequestInfo: head_repo=str(head_repo.get("full_name") or "") if isinstance(head_repo, Mapping) else "", title=str(data.get("title") or ""), body=str(data.get("body") or ""), + head_sha=str(head.get("sha") or "") if isinstance(head, Mapping) else "", ) diff --git a/python/robomp/src/host_tools.py b/python/robomp/src/host_tools.py index 92cdd42be..9633866b7 100644 --- a/python/robomp/src/host_tools.py +++ b/python/robomp/src/host_tools.py @@ -57,6 +57,7 @@ _AGENT_HOME = Path("/srv/agent-home") _PRE_PR_FIX_TIMEOUT_SECONDS = 600.0 _PRE_PR_CHECK_TIMEOUT_SECONDS = 600.0 _PRE_PR_CHECK_MAX_OUTPUT = 12_000 +_DIFF_HUNK_RE = re.compile(r"^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@") @dataclass(slots=True) @@ -1741,6 +1742,85 @@ def _build_pr_review_comment(bindings: ToolBindings) -> HostTool[Any, Any]: ) +def _diff_anchorable_lines(patch: str) -> tuple[frozenset[int], frozenset[int]]: + """Map a unified-diff patch to (RIGHT, LEFT) anchorable line sets. + + RIGHT = new-file line numbers of added (+) lines and in-hunk context + lines; LEFT = old-file line numbers of deleted (-) lines and in-hunk + context lines. GitHub only anchors review comments on lines inside a + hunk; a comment elsewhere 422s the whole review. + """ + right: set[int] = set() + left: set[int] = set() + new_line: int | None = None + old_line: int | None = None + for raw in patch.splitlines(): + m = _DIFF_HUNK_RE.match(raw) + if m: + old_line = int(m.group(1)) + new_line = int(m.group(2)) + continue + if raw.startswith("\\"): + continue + # `+++`/`---` are file headers only before the first hunk. Inside a + # hunk a diff line's *content* may start with `++` (added) or `--` + # (removed), and those lines must advance the counters. + in_hunk = new_line is not None and old_line is not None + if raw.startswith(("+++", "---")) and not in_hunk: + continue + if not in_hunk: + continue + if raw.startswith("+"): + right.add(new_line) + new_line += 1 + elif raw.startswith("-"): + left.add(old_line) + old_line += 1 + else: + right.add(new_line) + left.add(old_line) + new_line += 1 + old_line += 1 + return frozenset(right), frozenset(left) + + +def _filter_anchorable_comments(staged: list[Any], files: list[PullRequestFileInfo]) -> tuple[list[Any], list[Any]]: + """Partition staged comments into (anchorable, dropped) by diff hunk membership.""" + by_path = {f.path: f for f in files} + cache: dict[str, tuple[frozenset[int], frozenset[int]]] = {} + anchorable: list[Any] = [] + dropped: list[Any] = [] + for c in staged: + entry = by_path.get(c.path) + if entry is None: + dropped.append(c) # path not in the PR diff (stale/renamed path) + continue + if entry.path not in cache: + cache[entry.path] = _diff_anchorable_lines(entry.patch) + right, left = cache[entry.path] + if not entry.patch: + # Platform omitted the patch (binary file, no-op, API gap) — + # fail open per handoff §3c; a genuine rejection is caught by + # the 422/500 fallback after submit. + anchorable.append(c) + continue + side = str(c.side or "RIGHT").upper() + lines = right if side == "RIGHT" else left + if c.start_line is not None: + start_side = str(c.start_side or side).upper() + if start_side != side or c.start_line > c.line: + dropped.append(c) + continue + if c.start_line not in lines or c.line not in lines: + dropped.append(c) + continue + elif c.line not in lines: + dropped.append(c) + continue + anchorable.append(c) + return anchorable, dropped + + def _build_submit_pr_review(bindings: ToolBindings) -> HostTool[Any, Any]: def execute(args: dict[str, Any], _ctx: HostToolContext[Any]) -> str: _require_review_mode(bindings, "submit_pr_review", args) @@ -1751,27 +1831,133 @@ def _build_submit_pr_review(bindings: ToolBindings) -> HostTool[Any, Any]: _raise_command(msg) staged = bindings.db.list_staged_review_comments(bindings.issue_key) comments = [_review_comment_to_payload(comment) for comment in staged] + # commit_id is only needed by Forgejo to anchor inline review comments. + # GitHub's reviews endpoint ignores it, so skip the extra API call. + commit_id: str | None = None + if getattr(bindings.github, "_platform", "github") == "forgejo": + try: + pr = _run_coro( + bindings.loop, + bindings.github.get_pull_request( + repo=bindings.repo.full_name, + number=bindings.default_comment_number, + ), + ) + commit_id = pr.head_sha or None + except GitHubError: + commit_id = None + body = body.strip() + dropped: list[Any] = [] + if staged: + try: + pr_files = _run_coro( + bindings.loop, + bindings.github.list_pr_files( + repo=bindings.repo.full_name, + pr_number=bindings.default_comment_number, + ), + ) + except GitHubError as exc: + # Fail open: patch fetch failed, submit unfiltered — the + # 422/500 fallback below still catches a bad batch. + _audit( + bindings, + "submit_pr_review", + args, + error=f"anchor validation skipped: {exc.status} {exc.message}", + ) + else: + filtered, dropped = _filter_anchorable_comments(staged, pr_files) + if dropped: + _audit( + bindings, + "submit_pr_review", + args, + result={"dropped": [f"{c.path}:{c.line}" for c in dropped]}, + ) + body += "\n\n## Not anchored to diff" + for c in dropped: + body += f"\n- **`{c.path}:{c.line}`** — {c.body}" + comments = [_review_comment_to_payload(c) for c in filtered] try: review = _run_coro( bindings.loop, bindings.github.submit_pr_review( repo=bindings.repo.full_name, pr_number=bindings.default_comment_number, - body=body.strip(), + body=body, event="COMMENT", comments=comments, + commit_id=commit_id, ), ) except GitHubError as exc: _audit(bindings, "submit_pr_review", args, error=str(exc)) - _raise_command(f"GitHub rejected PR review: {exc.status} {exc.message}") + # 422 = validation rejection (e.g. Forgejo can't anchor inline + # comments). 500 = Forgejo internal error on the reviews endpoint + # (observed on certain PRs — the server crashes instead of returning + # a proper 422). Both mean the batched review can't land as-is. + # Degrade to visible issue comments (mirrors mira) so findings still + # surface and the model doesn't retry and degrade its own output. + # Without this fallback, a 500 propagates to the model as a raw + # error, triggering a retry-and-simplify loop where the model + # strips newlines from its review body on subsequent attempts. + if exc.status not in (422, 500): + _raise_command(f"GitHub rejected PR review: {exc.status} {exc.message}") + + def _note(comment: Any) -> str: + return f"**`{comment.path}:{comment.line}`**\n\n{comment.body}" + + posted_inline = 0 + try: + _run_coro( + bindings.loop, + bindings.github.post_comment(bindings.repo.full_name, bindings.default_comment_number, body), + ) + for comment in staged: + _run_coro( + bindings.loop, + bindings.github.post_comment( + bindings.repo.full_name, bindings.default_comment_number, _note(comment) + ), + ) + posted_inline += 1 + except GitHubError as fexc: + _audit(bindings, "submit_pr_review", args, error=str(fexc)) + _raise_command( + f"Review rejected ({exc.status}) and fallback comment posting failed: {fexc.status} {fexc.message}" + ) + + cleared = bindings.db.clear_staged_review_comments(bindings.issue_key) + _audit( + bindings, + "submit_pr_review", + args, + result={ + "fallback": "issue_comments", + "summary": True, + "inline": posted_inline, + "cleared": cleared, + }, + ) + return ( + f"review rejected ({exc.status}); posted summary + {posted_inline} inline comment(s) as issue comments" + ) cleared = bindings.db.clear_staged_review_comments(bindings.issue_key) _audit( bindings, "submit_pr_review", args, - result={"review_id": review.id, "comments": len(comments), "cleared": cleared, "event": "COMMENT"}, + result={ + "review_id": review.id, + "comments": len(comments), + "dropped": len(dropped), + "cleared": cleared, + "event": "COMMENT", + }, ) + if dropped: + return f"submitted PR review id={review.id}; comments={len(comments)}; dropped={len(dropped)} not anchored to diff" return f"submitted PR review id={review.id}; comments={len(comments)}" return host_tool( diff --git a/python/robomp/src/proxy/server.py b/python/robomp/src/proxy/server.py index 2d3bd96a6..76c8d90b2 100644 --- a/python/robomp/src/proxy/server.py +++ b/python/robomp/src/proxy/server.py @@ -671,6 +671,8 @@ def create_proxy_app(settings: Settings) -> FastAPI: body = _require_str(data.get("body"), "body") event = str(data.get("event") or "COMMENT") comments = _require_review_comments(data.get("comments")) + commit_id_raw = data.get("commit_id") + commit_id = commit_id_raw if isinstance(commit_id_raw, str) and commit_id_raw else None github: GitHubClient = request.app.state.github try: review = await github.submit_pr_review( @@ -679,6 +681,7 @@ def create_proxy_app(settings: Settings) -> FastAPI: body=body, event=event, comments=comments, + commit_id=commit_id, ) except GitHubError as exc: return _gh_error_response(exc) diff --git a/python/robomp/src/proxy_client.py b/python/robomp/src/proxy_client.py index ddf0c3229..c38c2ba0d 100644 --- a/python/robomp/src/proxy_client.py +++ b/python/robomp/src/proxy_client.py @@ -334,17 +334,21 @@ class GitHubProxyClient: body: str, event: str, comments: list[Mapping[str, Any]], + commit_id: str | None = None, ) -> PullRequestReviewInfo: + json_body: dict[str, Any] = { + "repo": repo, + "pr_number": pr_number, + "body": body, + "event": event, + "comments": comments, + } + if commit_id: + json_body["commit_id"] = commit_id data = await self._request( "POST", "/gh/v1/submit_pr_review", - json_body={ - "repo": repo, - "pr_number": pr_number, - "body": body, - "event": event, - "comments": comments, - }, + json_body=json_body, ) return _pr_review_from(data) @@ -602,6 +606,7 @@ def _pr_file_from(data: Any) -> PullRequestFileInfo: status=str(data.get("status") or ""), additions=int(data.get("additions") or 0), deletions=int(data.get("deletions") or 0), + patch=str(data.get("patch") or ""), ) @@ -619,6 +624,7 @@ def _pr_from(data: Any) -> PullRequestInfo: head_repo=str(data.get("head_repo") or ""), title=str(data.get("title") or ""), body=str(data.get("body") or ""), + head_sha=str(data.get("head_sha") or ""), ) diff --git a/python/robomp/tests/test_github_client.py b/python/robomp/tests/test_github_client.py index 0ed67681c..3b5dc5450 100644 --- a/python/robomp/tests/test_github_client.py +++ b/python/robomp/tests/test_github_client.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import json import httpx import pytest @@ -123,7 +124,11 @@ def test_get_pull_request_parses_head_repo_and_author() -> None: json={ "number": 9, "html_url": "https://github.com/octo/widget/pull/9", - "head": {"ref": "farm/abc12345/fix", "repo": {"full_name": "octo/widget"}}, + "head": { + "ref": "farm/abc12345/fix", + "sha": "abc1234567890123456789012345678901234567", + "repo": {"full_name": "octo/widget"}, + }, "base": {"ref": "main"}, "state": "open", "user": {"login": "robomp-bot"}, @@ -133,6 +138,7 @@ def test_get_pull_request_parses_head_repo_and_author() -> None: client = GitHubClient("tok", transport=httpx.MockTransport(handler)) pr = _run_async(client.get_pull_request("octo/widget", 9)) assert pr.head_ref == "farm/abc12345/fix" + assert pr.head_sha == "abc1234567890123456789012345678901234567" assert pr.head_repo == "octo/widget" assert pr.author == "robomp-bot" @@ -166,7 +172,15 @@ def test_list_pr_files_parses_changed_file_summary() -> None: assert request.url.params.get("per_page") == "100" return httpx.Response( 200, - json=[{"filename": "src/app.py", "status": "modified", "additions": 5, "deletions": 2}], + json=[ + { + "filename": "src/app.py", + "status": "modified", + "additions": 5, + "deletions": 2, + "patch": "@@ -8,3 +8,5 @@\n ctx\n+added\n ctx2", + } + ], ) client = GitHubClient("tok", transport=httpx.MockTransport(handler)) @@ -175,6 +189,20 @@ def test_list_pr_files_parses_changed_file_summary() -> None: assert files[0].path == "src/app.py" assert files[0].additions == 5 assert files[0].deletions == 2 + assert files[0].patch.startswith("@@ -8,3 +8,5") + + +def test_list_pr_files_defaults_missing_patch_to_empty() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/repos/octo/widget/pulls/9/files" + return httpx.Response( + 200, + json=[{"filename": "src/app.py", "status": "modified", "additions": 5, "deletions": 2}], + ) + + client = GitHubClient("tok", transport=httpx.MockTransport(handler)) + files = _run_async(client.list_pr_files("octo/widget", 9)) + assert files[0].patch == "" def test_list_pr_files_paginates_past_first_page() -> None: @@ -248,6 +276,48 @@ def test_submit_pr_review_posts_comment_event_and_inline_comments() -> None: } +def test_submit_pr_review_forgejo_uses_new_position_payload() -> None: + captured: dict[str, object] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["path"] = request.url.path + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "summary", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + + client = GitHubClient("tok", transport=httpx.MockTransport(handler), platform="forgejo") + review = _run_async( + client.submit_pr_review( + repo="octo/widget", + pr_number=9, + body="summary", + event="COMMENT", + comments=[ + {"path": "src/app.py", "line": 12, "side": "RIGHT", "body": "finding"}, + {"path": "src/old.py", "line": 5, "side": "LEFT", "body": "removed-line finding"}, + ], + ) + ) + assert review.id == 44 + assert captured["path"] == "/repos/octo/widget/pulls/9/reviews" + assert captured["body"] == { + "body": "summary", + "event": "COMMENT", + "comments": [ + {"path": "src/app.py", "body": "finding", "new_position": 12}, + {"path": "src/old.py", "body": "removed-line finding", "old_position": 5}, + ], + } + + def test_204_no_content_returns_none() -> None: transport = httpx.MockTransport(lambda r: httpx.Response(204)) client = GitHubClient("tok", transport=transport) diff --git a/python/robomp/tests/test_host_tools.py b/python/robomp/tests/test_host_tools.py index 3e2f2f541..ecd7f8063 100644 --- a/python/robomp/tests/test_host_tools.py +++ b/python/robomp/tests/test_host_tools.py @@ -62,6 +62,20 @@ def _stub_repo() -> RepoInfo: ) +# Unified diff whose anchorable lines are RIGHT {10..14} / LEFT {9..12}; +# line 15+ is a gap (unanchorable) and RIGHT line 9 is before the hunk. +_PATCH = "@@ -9,5 +10,6 @@ def f():\n ctx1\n-old10\n+new11\n ctx2\n+new13\n ctx3\n" + + +def _pr_files_response(request: httpx.Request, patch: str = _PATCH, *, status: int = 200) -> httpx.Response: + if status != 200: + return httpx.Response(status, json={"message": "files fetch failed"}) + return httpx.Response( + 200, + json=[{"filename": "src/app.py", "status": "modified", "additions": 1, "deletions": 1, "patch": patch}], + ) + + def _make_loop_in_background() -> tuple[asyncio.AbstractEventLoop, threading.Thread]: loop = asyncio.new_event_loop() t = threading.Thread(target=loop.run_forever, daemon=True) @@ -1112,9 +1126,13 @@ def _pr_bindings( def _review_bindings( - db: Database, tmp_path: Path, transport: httpx.MockTransport + db: Database, + tmp_path: Path, + transport: httpx.MockTransport, + *, + platform: str = "github", ) -> tuple[ToolBindings, asyncio.AbstractEventLoop, threading.Thread]: - github = GitHubClient("token", transport=transport) + github = GitHubClient("token", transport=transport, platform=platform) loop, thread = _make_loop_in_background() issue = IssueInfo( repo="octo/widget", @@ -1234,6 +1252,8 @@ def test_pr_review_comment_stages_and_submit_flushes_one_comment_review(db: Data captured: dict[str, Any] = {} def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) if request.url.path.endswith("/reviews"): captured["path"] = request.url.path captured["body"] = json.loads(request.content) @@ -1315,11 +1335,12 @@ def test_submit_pr_review_posts_summary_only_when_no_staged_comments(db: Databas def test_submit_pr_review_failure_keeps_staged_comments(db: Database, tmp_path: Path) -> None: - bindings, loop, t = _review_bindings( - db, - tmp_path, - httpx.MockTransport(lambda _request: httpx.Response(422, json={"message": "Validation failed"})), - ) + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + return httpx.Response(403, json={"message": "forbidden"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) try: stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") @@ -1334,6 +1355,533 @@ def test_submit_pr_review_failure_keeps_staged_comments(db: Database, tmp_path: assert rows[0].path == "src/app.py" +def test_submit_pr_review_drops_unanchorable_comment_and_folds_into_summary(db: Database, tmp_path: Path) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": captured["body"]["body"], + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "in-hunk finding"}, _ctx()) + stage_tool.execute({"path": "src/app.py", "line": 15, "body": "gap finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert captured["body"]["body"] == "summary\n\n## Not anchored to diff\n- **`src/app.py:15`** — gap finding" + assert captured["body"]["comments"] == [ + {"path": "src/app.py", "line": 12, "side": "RIGHT", "body": "in-hunk finding"} + ] + assert "submitted PR review" in result + assert "dropped=1" in result + assert db.list_staged_review_comments(bindings.issue_key) == [] + + +def test_submit_pr_review_drops_all_comments_submits_summary_only(db: Database, tmp_path: Path) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 15, "body": "gap finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert captured["body"]["comments"] == [] + assert captured["body"]["body"].endswith("## Not anchored to diff\n- **`src/app.py:15`** — gap finding") + assert "comments=0" in result + assert "dropped=1" in result + assert db.list_staged_review_comments(bindings.issue_key) == [] + + +def test_submit_pr_review_drops_comment_for_path_not_in_diff(db: Database, tmp_path: Path) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/other.py", "line": 3, "body": "stale path finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert captured["body"]["comments"] == [] + assert captured["body"]["body"].endswith("## Not anchored to diff\n- **`src/other.py:3`** — stale path finding") + assert "dropped=1" in result + assert db.list_staged_review_comments(bindings.issue_key) == [] + + +def test_submit_pr_review_skips_validation_when_files_fetch_fails(db: Database, tmp_path: Path) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request, status=500) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 15, "body": "gap finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert captured["body"]["comments"] == [{"path": "src/app.py", "line": 15, "side": "RIGHT", "body": "gap finding"}] + assert captured["body"]["body"] == "summary" + assert "comments=1" in result + assert "dropped" not in result + + +def test_submit_pr_review_422_falls_back_to_issue_comments(db: Database, tmp_path: Path) -> None: + comment_bodies: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + return httpx.Response(422, json={"message": "Validation failed"}) + if request.url.path == "/repos/octo/widget/issues/99/comments": + body = json.loads(request.content)["body"] + comment_bodies.append(body) + return httpx.Response(200, json={"id": len(comment_bodies), "body": body}) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "posted summary + 1 inline comment(s) as issue comments" in result + assert comment_bodies == ["summary", "**`src/app.py:12`**\n\nfinding"] + assert db.list_staged_review_comments(bindings.issue_key) == [] + + +def test_submit_pr_review_500_falls_back_to_issue_comments(db: Database, tmp_path: Path) -> None: + """A 500 from Forgejo's reviews endpoint triggers the same fallback as 422. + + Without this, the 500 propagates to the model, causing a retry-and-degrade + loop where the model strips newlines from subsequent review bodies. + """ + comment_bodies: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + return httpx.Response(500, json={"message": "github error"}) + if request.url.path == "/repos/octo/widget/issues/99/comments": + body = json.loads(request.content)["body"] + comment_bodies.append(body) + return httpx.Response(200, json={"id": len(comment_bodies), "body": body}) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "posted summary + 1 inline comment(s) as issue comments" in result + assert comment_bodies == ["summary", "**`src/app.py:12`**\n\nfinding"] + assert db.list_staged_review_comments(bindings.issue_key) == [] + + +def test_submit_pr_review_range_requires_both_endpoints_anchorable(db: Database, tmp_path: Path) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + # Reversed range: start_line (15) > line (14) — dropped. + stage_tool.execute( + { + "path": "src/app.py", + "line": 14, + "start_line": 15, + "start_side": "RIGHT", + "side": "RIGHT", + "body": "reversed range", + }, + _ctx(), + ) + # Valid range: both endpoints (10, 12) are context lines in the hunk — kept. + stage_tool.execute( + { + "path": "src/app.py", + "line": 12, + "start_line": 10, + "start_side": "RIGHT", + "side": "RIGHT", + "body": "valid range", + }, + _ctx(), + ) + # Cross-side range: start_side LEFT with side RIGHT — dropped. + stage_tool.execute( + { + "path": "src/app.py", + "line": 11, + "start_line": 10, + "start_side": "LEFT", + "side": "RIGHT", + "body": "cross-side range", + }, + _ctx(), + ) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert captured["body"]["comments"] == [ + { + "path": "src/app.py", + "line": 12, + "side": "RIGHT", + "body": "valid range", + "start_line": 10, + "start_side": "RIGHT", + } + ] + assert "src/app.py:14" in captured["body"]["body"] + assert "src/app.py:11" in captured["body"]["body"] + assert "dropped=2" in result + + +def test_submit_pr_review_forgejo_fetches_commit_id(db: Database, tmp_path: Path) -> None: + """Forgejo anchors inline comments by commit: submit must fetch the PR and + include the head sha as commit_id in the reviews POST body.""" + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99": + return httpx.Response( + 200, + json={ + "number": 99, + "html_url": "https://x/octo/widget/pull/99", + "head": {"ref": "fix-crash", "sha": "abc123456789"}, + "base": {"ref": "main"}, + "state": "open", + "user": {"login": "alice"}, + }, + ) + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler), platform="forgejo") + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "in-hunk finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "submitted PR review" in result + assert captured["body"]["commit_id"] == "abc123456789" + + +def test_submit_pr_review_forgejo_commit_id_fetch_failure_is_swallowed( + db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """When the PR fetch fails the commit_id is omitted (fail open) and the + review still submits without it.""" + monkeypatch.setattr(GitHubClient, "_TRANSIENT_RETRY_DELAYS", (0.01, 0.01)) + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99": + return httpx.Response(500, json={"message": "internal error"}) + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler), platform="forgejo") + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "in-hunk finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "submitted PR review" in result + assert "commit_id" not in captured["body"] + + +def test_submit_pr_review_422_and_fallback_comment_failure_raises_and_keeps_staged( + db: Database, + tmp_path: Path, +) -> None: + """When the reviews endpoint AND the issue-comments fallback both fail, the + tool raises and the staged comments survive for a later retry.""" + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + return httpx.Response(422, json={"message": "Validation failed"}) + if request.url.path == "/repos/octo/widget/issues/99/comments": + return httpx.Response(500, json={"message": "internal error"}) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "finding"}, _ctx()) + with pytest.raises(RpcCommandError, match="fallback comment posting failed"): + submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + rows = db.list_staged_review_comments(bindings.issue_key) + assert len(rows) == 1 + assert rows[0].path == "src/app.py" + + +def test_submit_pr_review_empty_patch_fails_open(db: Database, tmp_path: Path) -> None: + """A file whose patch the platform omitted is not a rejection reason: the + comment is kept rather than folded into a 'Not anchored to diff' section.""" + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request, patch="") + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 12, "body": "binary finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "submitted PR review" in result + assert "dropped" not in result + assert captured["body"]["comments"] == [ + {"path": "src/app.py", "line": 12, "side": "RIGHT", "body": "binary finding"} + ] + assert "Not anchored to diff" not in captured["body"]["body"] + + +def test_submit_pr_review_left_side_single_line_anchoring(db: Database, tmp_path: Path) -> None: + """LEFT-side comments anchor against the old file's hunk lines + (_PATCH: LEFT {9..12}): an in-hunk line is kept, an out-of-hunk line is + dropped and folded into the summary.""" + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/octo/widget/pulls/99/files": + return _pr_files_response(request) + if request.url.path.endswith("/reviews"): + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": 44, + "user": {"login": "robomp-bot"}, + "body": "ok", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + bindings, loop, t = _review_bindings(db, tmp_path, httpx.MockTransport(handler)) + try: + stage_tool = next(x for x in build(bindings) if x.name == "pr_review_comment") + submit_tool = next(x for x in build(bindings) if x.name == "submit_pr_review") + stage_tool.execute({"path": "src/app.py", "line": 10, "side": "LEFT", "body": "old line finding"}, _ctx()) + stage_tool.execute({"path": "src/app.py", "line": 13, "side": "LEFT", "body": "gap finding"}, _ctx()) + result = submit_tool.execute({"body": "summary"}, _ctx()) + finally: + _stop_loop(loop, t) + + assert "dropped=1" in result + assert captured["body"]["comments"] == [ + {"path": "src/app.py", "line": 10, "side": "LEFT", "body": "old line finding"} + ] + assert "## Not anchored to diff" in captured["body"]["body"] + assert "**`src/app.py:13`** — gap finding" in captured["body"]["body"] + + +def test_diff_anchorable_lines_parses_hunk_sides() -> None: + right, left = host_tools._diff_anchorable_lines(_PATCH) + assert right == frozenset({10, 11, 12, 13, 14}) + assert left == frozenset({9, 10, 11, 12}) + + # Gap between hunks: line 30 in the old file / 29..30 in the new file are + # unanchorable (the PR 1111 smtp.go:106 failure shape). + right, left = host_tools._diff_anchorable_lines("@@ -1,2 +1,3 @@\n a\n+x\n b\n@@ -30,2 +31,2 @@\n c\n d\n") + assert right >= frozenset({1, 2, 3, 31, 32}) + assert right.isdisjoint(range(4, 31)) + + assert host_tools._diff_anchorable_lines("") == (frozenset(), frozenset()) + assert host_tools._diff_anchorable_lines("\0Binary files differ") == (frozenset(), frozenset()) + + right, left = host_tools._diff_anchorable_lines("@@ -9 +10 @@ x\n+a\n b\n") + assert right == frozenset({10, 11}) + assert left == frozenset({9}) + + # `\ No newline at end of file` markers are ignored. + right, _ = host_tools._diff_anchorable_lines("@@ -1,2 +1,3 @@\n a\n+b\n\\ No newline at end of file\n") + assert right == frozenset({1, 2}) + + # File-creation hunk boundary: everything is added, nothing exists on the + # LEFT; file-deletion hunk: everything is removed, RIGHT is empty. + right, left = host_tools._diff_anchorable_lines("@@ -0,0 +1,2 @@\n+a\n+b\n") + assert right == frozenset({1, 2}) + assert left == frozenset() + + right, left = host_tools._diff_anchorable_lines("@@ -5,3 +5,0 @@\n-a\n-b\n-c\n") + assert right == frozenset() + assert left == frozenset({5, 6, 7}) + + +def test_diff_anchorable_lines_in_hunk_plus_minus_content() -> None: + """Added lines whose content starts with `++` and removed lines starting + with `--` must be treated as diff content, not file headers — otherwise + the line counters desync (the `+++ b/...` skip used to swallow them).""" + patch = "+++ b/src/app.py\n--- a/src/app.py\n@@ -1,2 +1,3 @@\n ctx\n tail\n--- removed\n+++ added\n" + # Old file: ctx(1), tail(2), --- removed(3). New file: ctx(1), tail(2), +++ added(3). + right, left = host_tools._diff_anchorable_lines(patch) + assert right == frozenset({1, 2, 3}) + assert left == frozenset({1, 2, 3}) + + def test_review_tools_reject_outside_review_mode(db: Database, tmp_path: Path) -> None: bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500))) try: diff --git a/python/robomp/tests/test_proxy_client.py b/python/robomp/tests/test_proxy_client.py index ff9c03eb1..ec33a94c9 100644 --- a/python/robomp/tests/test_proxy_client.py +++ b/python/robomp/tests/test_proxy_client.py @@ -31,7 +31,7 @@ from robomp.github_client import ( ) from robomp.proxy.server import create_proxy_app from robomp.proxy_client import GitHubProxyClient, ProxyGitTransport -from robomp.proxy_hmac import HEADER_SIGNATURE, HEADER_TIMESTAMP, verify +from robomp.proxy_hmac import HEADER_SIGNATURE, HEADER_TIMESTAMP, sign, verify from robomp.sandbox import workspace_key _HMAC = "test-hmac-key-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" @@ -316,7 +316,15 @@ def round_trip_app(proxy_settings: Settings): if path == "/repos/octo/widget/pulls/2/files": return httpx.Response( 200, - json=[{"filename": "src/app.py", "status": "modified", "additions": 2, "deletions": 1}], + json=[ + { + "filename": "src/app.py", + "status": "modified", + "additions": 2, + "deletions": 1, + "patch": "@@ -8,5 +8,6 @@\n ctx\n-old\n+new\n", + } + ], ) if path == "/repos/octo/widget/pulls/2/reviews" and req.method == "POST": body = json.loads(req.content) @@ -340,7 +348,11 @@ def round_trip_app(proxy_settings: Settings): json={ "number": 4, "html_url": "https://example/4", - "head": {"ref": "feat", "repo": {"full_name": "octo/widget"}}, + "head": { + "ref": "feat", + "sha": "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + "repo": {"full_name": "octo/widget"}, + }, "base": {"ref": "main"}, "state": "open", "user": {"login": "robomp-bot"}, @@ -405,6 +417,7 @@ async def test_round_trip_all_endpoints(round_trip_app) -> None: files = await client.list_pr_files("octo/widget", 2) assert len(files) == 1 and isinstance(files[0], PullRequestFileInfo) assert files[0].path == "src/app.py" + assert files[0].patch == "@@ -8,5 +8,6 @@\n ctx\n-old\n+new\n" submitted = await client.submit_pr_review( repo="octo/widget", @@ -420,6 +433,7 @@ async def test_round_trip_all_endpoints(round_trip_app) -> None: existing_pr = await client.get_pull_request("octo/widget", 4) assert isinstance(existing_pr, PullRequestInfo) assert existing_pr.head_ref == "feat" + assert existing_pr.head_sha == "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" assert existing_pr.author == "robomp-bot" posted = await client.post_comment("octo/widget", 1, "hi") @@ -486,6 +500,144 @@ async def test_close_issue_round_trip(proxy_settings: Settings) -> None: assert captured["body"] == {"state": "closed", "state_reason": "completed"} +def _capturing_app(app, path: str) -> tuple[Callable, list[dict[str, object]]]: + """ASGI wrapper recording raw JSON bodies POSTed to `path`.""" + bodies: list[dict[str, object]] = [] + + async def middleware(scope, receive, send): + if scope["type"] == "http" and scope.get("path") == path: + raw = b"" + while True: + message = await receive() + raw += message.get("body", b"") + if not message.get("more_body", False): + break + bodies.append(json.loads(raw)) + + async def replay() -> dict[str, object]: + return {"type": "http.request", "body": raw, "more_body": False} + + return await app(scope, replay, send) + return await app(scope, receive, send) + + return middleware, bodies + + +async def test_submit_pr_review_commit_id_reaches_wire(proxy_settings: Settings) -> None: + """commit_id must appear in the /gh/v1/submit_pr_review wire body the + proxy client POSTs, and the server must forward it to the direct client.""" + app = create_proxy_app(proxy_settings) + app.state.settings = proxy_settings + upstream: dict[str, object] = {} + + def gh(req: httpx.Request) -> httpx.Response: + if req.url.path == "/repos/octo/widget/pulls/2/reviews" and req.method == "POST": + upstream["body"] = json.loads(req.content) + return httpx.Response( + 200, + json={ + "id": 55, + "user": {"login": "robomp-bot"}, + "body": "summary", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + _attach_gh(app, gh) + middleware, wire_bodies = _capturing_app(app, "/gh/v1/submit_pr_review") + client = GitHubProxyClient( + base_url="http://proxy.test", + hmac_key=_HMAC, + transport=httpx.ASGITransport(app=middleware), + ) + review = await client.submit_pr_review( + repo="octo/widget", + pr_number=2, + body="summary", + event="COMMENT", + comments=[{"path": "src/app.py", "line": 12, "side": "RIGHT", "body": "finding"}], + commit_id="abc123", + ) + assert review.id == 55 + assert wire_bodies == [ + { + "repo": "octo/widget", + "pr_number": 2, + "body": "summary", + "event": "COMMENT", + "comments": [{"path": "src/app.py", "line": 12, "side": "RIGHT", "body": "finding"}], + "commit_id": "abc123", + } + ] + # Server forwarded it to the direct client, which put it on the GitHub wire. + assert upstream["body"]["commit_id"] == "abc123" + + # Without commit_id the key is omitted from the proxy wire body. + await client.submit_pr_review( + repo="octo/widget", + pr_number=2, + body="summary", + event="COMMENT", + comments=[], + ) + assert "commit_id" not in wire_bodies[1] + + +@pytest.mark.parametrize("bad_commit_id", [12345, ""]) +async def test_submit_pr_review_rejects_non_string_commit_id(proxy_settings: Settings, bad_commit_id: object) -> None: + """The proxy server must not forward a non-string or empty commit_id + upstream: a raw POST bypasses the proxy client, so assert on the + upstream-captured reviews body — the key must be absent and the request + still succeeds.""" + app = create_proxy_app(proxy_settings) + app.state.settings = proxy_settings + upstream: dict[str, object] = {} + + def gh(req: httpx.Request) -> httpx.Response: + if req.url.path == "/repos/octo/widget/pulls/3/reviews" and req.method == "POST": + upstream["body"] = json.loads(req.content) + return httpx.Response( + 200, + json={ + "id": 56, + "user": {"login": "robomp-bot"}, + "body": "summary", + "state": "COMMENTED", + "submitted_at": "t", + }, + ) + return httpx.Response(404, json={"message": "unrouted"}) + + app.state.github = GitHubClient(_TOKEN, transport=httpx.MockTransport(gh)) + payload = { + "repo": "octo/widget", + "pr_number": 3, + "body": "summary", + "event": "COMMENT", + "comments": [], + "commit_id": bad_commit_id, + } + body = json.dumps(payload).encode() + timestamp, sig = sign(method="POST", path="/gh/v1/submit_pr_review", body=body, key=_HMAC_BYTES) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), + base_url="http://proxy.test", + ) as client: + resp = await client.post( + "/gh/v1/submit_pr_review", + content=body, + headers={ + HEADER_TIMESTAMP: timestamp, + HEADER_SIGNATURE: sig, + "Content-Type": "application/json", + }, + ) + assert resp.status_code == 200, resp.text + assert "commit_id" not in upstream["body"] + + # ============================================================================ # 3. Error decode # ============================================================================