feat: report Open SWE Review as a PR check run (#1484)

* feat: report Open SWE Review as a PR check run

Auto-review dispatch now creates an in-progress 'Open SWE Review' check run
on the PR head SHA; publish_review completes it (neutral with findings,
success when clean). An after-agent hook fails the check if the run dies
before publishing. Requires the GitHub App's Checks: Read & write permission;
all calls are best-effort so a missing permission never breaks reviews.

* fix: address review feedback on check-run settling

Keep review_check_run_id when the completion PATCH fails so a later
publish or the after-agent hook can retry instead of hanging the check;
count out-of-diff findings toward the check conclusion.

* fix: retry failed check completion with the real publish conclusion

A transient PATCH failure after a successful publish previously left the
check id for the after-agent hook, which settled it as 'failure'. Persist
the intended result as review_check_pending_result and have the hook
prefer it over the generic failure fallback.

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Johannes du Plessis 2026-06-10 11:53:48 -07:00 • committed by GitHub
parent 370644ae8b
commit e8770e1004
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 550 additions and 0 deletions

View file

@ -76,6 +76,7 @@ Write this down. You'll use it in the callback URL below and again in step 4 whe
- Contents: Read & write
- Pull requests: Read & write
- Issues: Read & write
- Checks: Read & write — reports an "Open SWE Review" check run on PRs while an auto-review runs. Without it, check-run creation fails (logged, best-effort) but reviews still work.
- Metadata: Read-only
- **Organization permissions** (required only if you plan to set `ALLOWED_GITHUB_ORGS` — see step 5 / Security):
- Members: Read-only — used to verify org membership for the dashboard-login gate via `GET /orgs/{org}/memberships/{username}`. Without this permission that call returns 403, the check fails closed, and **every** dashboard login is rejected.

View file

@ -7,6 +7,7 @@ from .refresh_slack_status import SlackAssistantStatusMiddleware
from .sandbox_circuit_breaker import SandboxCircuitBreakerMiddleware
from .sanitize_thinking_blocks import SanitizeThinkingBlocksMiddleware
from .sanitize_tool_inputs import SanitizeToolInputsMiddleware
from .settle_review_check import settle_review_check_on_exit
from .tool_error_handler import ToolErrorMiddleware
__all__ = [
@ -20,4 +21,5 @@ __all__ = [
"check_message_queue_before_model",
"ensure_no_empty_msg",
"notify_step_limit_reached",
"settle_review_check_on_exit",
]

View file

@ -0,0 +1,84 @@
"""After-agent middleware that closes a still-open review check run.
``publish_review`` normally completes the ``Open SWE Review`` check run and
clears ``review_check_run_id`` from reviewer thread metadata. If the run ends
without ever publishing (crash, model-call limit, sandbox failure), the check
would hang "in progress" on the PR forever. This hook closes it as a failure
so the PR's checks section reflects reality.
"""
from __future__ import annotations
import logging
from typing import Any
from langchain.agents.middleware import AgentState, after_agent
from langgraph.config import get_config
from langgraph.runtime import Runtime
from ..reviewer_findings import get_thread_metadata
from ..reviewer_publish import settle_review_check_run
from ..utils.github_token import get_github_token
logger = logging.getLogger(__name__)
@after_agent
async def settle_review_check_on_exit(
state: AgentState,
runtime: Runtime,
) -> dict[str, Any] | None:
"""Fail the tracked review check run if the run ended without publishing."""
config = get_config()
configurable = config.get("configurable", {})
if not isinstance(configurable, dict):
return None
thread_id = configurable.get("thread_id")
repo_config = configurable.get("repo")
if not isinstance(thread_id, str) or not thread_id or not isinstance(repo_config, dict):
return None
owner = repo_config.get("owner")
repo = repo_config.get("name")
if not isinstance(owner, str) or not owner or not isinstance(repo, str) or not repo:
return None
try:
metadata = await get_thread_metadata(thread_id)
if not isinstance(metadata.get("review_check_run_id"), int):
return None
token = get_github_token()
if not token:
logger.warning("No GitHub token to settle stale review check on thread %s", thread_id)
return None
# A pending result means publish_review DID finish but its completion
# PATCH failed transiently — retry with the real conclusion instead of
# misreporting a published review as failed.
pending = metadata.get("review_check_pending_result")
if isinstance(pending, dict) and pending.get("conclusion") in {
"success",
"neutral",
"failure",
}:
conclusion = pending["conclusion"]
title = str(pending.get("title") or "Review completed")
summary = str(pending.get("summary") or "")
else:
conclusion = "failure"
title = "Review did not complete"
summary = (
"The Open SWE review run ended without publishing a review. "
"Re-trigger the review by pushing a commit or re-requesting it."
)
await settle_review_check_run(
thread_id=thread_id,
owner=owner,
repo=repo,
token=token,
conclusion=conclusion,
title=title,
summary=summary,
)
logger.info("Settled stale review check run for thread %s", thread_id)
except Exception:
logger.exception("Failed to settle stale review check run for thread %s", thread_id)
return None

View file

@ -38,6 +38,7 @@ from .middleware import (
SlackAssistantStatusMiddleware,
ToolErrorMiddleware,
check_message_queue_before_model,
settle_review_check_on_exit,
)
from .reviewer_diff import compute_diff_line_set, fetch_pr_diff, fetch_pr_metadata
from .reviewer_findings import (
@ -996,5 +997,6 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
check_message_queue_before_model,
SlackAssistantStatusMiddleware(),
SanitizeThinkingBlocksMiddleware(),
settle_review_check_on_exit,
],
).with_config(config)

View file

@ -33,6 +33,7 @@ from .reviewer_findings import (
set_reviewer_thread_metadata,
)
from .utils.dashboard_links import dashboard_thread_url
from .utils.github_checks import CheckConclusion, complete_review_check_run
from .utils.github_token import GitHubAuthError
logger = logging.getLogger(__name__)
@ -425,6 +426,58 @@ async def clear_review_started_comment(
await set_reviewer_thread_metadata(thread_id, extra={"status_comment_id": None})
async def settle_review_check_run(
*,
thread_id: str,
owner: str,
repo: str,
token: str,
conclusion: CheckConclusion,
title: str,
summary: str,
) -> None:
"""Complete the tracked ``Open SWE Review`` check run, if one is open.
The dispatching webhook stores ``review_check_run_id`` in reviewer thread
metadata when it creates the check. No-op when none is tracked. The id is
only cleared after a successful PATCH so a transient failure (timeout,
5xx, rate limit) leaves it in place for the after-agent hook or a later
publish to retry — otherwise the check would hang in-progress forever.
On failure the intended result is persisted as
``review_check_pending_result`` so the retry reports this conclusion, not
a generic failure that would misreport a published review.
"""
metadata = await get_thread_metadata(thread_id)
check_run_id = metadata.get("review_check_run_id")
if not isinstance(check_run_id, int):
return
ok = await complete_review_check_run(
owner=owner,
repo=repo,
check_run_id=check_run_id,
token=token,
conclusion=conclusion,
title=title,
summary=summary,
)
if ok:
await set_reviewer_thread_metadata(
thread_id,
extra={"review_check_run_id": None, "review_check_pending_result": None},
)
else:
await set_reviewer_thread_metadata(
thread_id,
extra={
"review_check_pending_result": {
"conclusion": conclusion,
"title": title,
"summary": summary,
}
},
)
async def open_swe_review_exists(
*,
owner: str,

View file

@ -40,9 +40,11 @@ from ..reviewer_publish import (
render_review_body,
reply_to_review_comment,
resolve_review_thread,
settle_review_check_run,
)
from ..reviewer_reconcile import reconcile_findings_with_review_threads
from ..utils.dashboard_links import dashboard_thread_url
from ..utils.github_checks import review_check_conclusion
from ..utils.github_token import (
GitHubAuthError,
get_github_token,
@ -322,6 +324,16 @@ async def _publish_review_async(
)
await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha)
await clear_review_started_comment(thread_id=thread_id, owner=owner, repo=repo, token=token)
conclusion, check_title, check_summary = review_check_conclusion(0)
await settle_review_check_run(
thread_id=thread_id,
owner=owner,
repo=repo,
token=token,
conclusion=conclusion,
title=check_title,
summary=check_summary,
)
return {
"success": True,
"review_id": None,
@ -498,6 +510,18 @@ async def _publish_review_async(
await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha)
await clear_review_started_comment(thread_id=thread_id, owner=owner, repo=repo, token=token)
conclusion, check_title, check_summary = review_check_conclusion(
len(inline_comments) + len(eligible_out_of_diff)
)
await settle_review_check_run(
thread_id=thread_id,
owner=owner,
repo=repo,
token=token,
conclusion=conclusion,
title=check_title,
summary=check_summary,
)
result: dict[str, Any] = {
"success": True,

View file

@ -0,0 +1,136 @@
"""GitHub Checks API helpers for the reviewer's PR check run.
A check run named ``Open SWE Review`` is created on the PR head SHA when an
auto-review is dispatched, so the PR's checks section shows the review as
in-progress. ``publish_review`` (or the after-agent fallback) completes it.
All calls are best-effort: check runs require the GitHub App's
``Checks: Read & write`` permission, and a missing permission must never
break review dispatch or publish.
"""
from __future__ import annotations
import logging
from datetime import UTC, datetime
from typing import Literal
import httpx
logger = logging.getLogger(__name__)
REVIEW_CHECK_RUN_NAME = "Open SWE Review"
_GITHUB_API_BASE = "https://api.github.com"
CheckConclusion = Literal["success", "neutral", "failure"]
def _github_headers(token: str) -> dict[str, str]:
return {
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
}
def _utc_now_iso() -> str:
return datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
async def create_review_check_run(
*,
owner: str,
repo: str,
head_sha: str,
token: str,
details_url: str | None = None,
) -> int | None:
"""Create an in-progress ``Open SWE Review`` check run on ``head_sha``.
Returns the check run id, or ``None`` on any failure (most commonly the
App lacking the Checks permission).
"""
payload: dict[str, object] = {
"name": REVIEW_CHECK_RUN_NAME,
"head_sha": head_sha,
"status": "in_progress",
"started_at": _utc_now_iso(),
"output": {
"title": "Review in progress",
"summary": "Open SWE is reviewing this pull request…",
},
}
if details_url:
payload["details_url"] = details_url
url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/check-runs"
try:
async with httpx.AsyncClient() as client:
response = await client.post(
url, headers=_github_headers(token), json=payload, timeout=30
)
response.raise_for_status()
except httpx.HTTPError:
logger.exception(
"Failed to create review check run for %s/%s@%s "
"(does the GitHub App have Checks: Read & write?)",
owner,
repo,
head_sha,
)
return None
data = response.json()
check_run_id = data.get("id") if isinstance(data, dict) else None
return check_run_id if isinstance(check_run_id, int) else None
async def complete_review_check_run(
*,
owner: str,
repo: str,
check_run_id: int,
token: str,
conclusion: CheckConclusion,
title: str,
summary: str,
) -> bool:
"""Mark a review check run as completed. Returns True on success."""
url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/check-runs/{check_run_id}"
payload = {
"status": "completed",
"conclusion": conclusion,
"completed_at": _utc_now_iso(),
"output": {"title": title, "summary": summary},
}
try:
async with httpx.AsyncClient() as client:
response = await client.patch(
url, headers=_github_headers(token), json=payload, timeout=30
)
response.raise_for_status()
except httpx.HTTPError:
logger.exception(
"Failed to complete review check run %s on %s/%s", check_run_id, owner, repo
)
return False
return True
def review_check_conclusion(surfaced_count: int) -> tuple[CheckConclusion, str, str]:
"""Map a publish result to (conclusion, title, summary).
Findings surfaced → ``neutral`` so the check never blocks merges; a clean
review → ``success``.
"""
if surfaced_count > 0:
issue_word = "issue" if surfaced_count == 1 else "issues"
return (
"neutral",
f"Found {surfaced_count} potential {issue_word}",
f"Open SWE surfaced {surfaced_count} potential {issue_word} on this pull request.",
)
return (
"success",
"No issues found",
"Open SWE reviewed this pull request and found no issues.",
)

View file

@ -55,10 +55,12 @@ from .utils.auth import (
resolve_github_token_from_email,
)
from .utils.comments import get_recent_comments
from .utils.dashboard_links import dashboard_thread_url
from .utils.github_app import (
get_github_app_installation_token,
get_github_app_installation_token_with_expiry,
)
from .utils.github_checks import create_review_check_run
from .utils.github_comments import (
OPEN_SWE_TAGS,
GitHubAuthError,
@ -2043,6 +2045,16 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou
await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True, head_sha=head_sha)
check_run_id = await create_review_check_run(
owner=repo_config.get("owner", ""),
repo=repo_config.get("name", ""),
head_sha=head_sha,
token=app_token,
details_url=dashboard_thread_url(thread_id),
)
if check_run_id is not None:
await set_reviewer_thread_metadata(thread_id, extra={"review_check_run_id": check_run_id})
is_re_review = bool(last_reviewed_sha)
if is_re_review:
prompt = (

236
tests/test_github_checks.py Normal file
View file

@ -0,0 +1,236 @@
from __future__ import annotations
from typing import Any
import httpx
import pytest
from agent import reviewer_publish
from agent.utils import github_checks
class _FakeResponse:
def __init__(self, payload: dict[str, Any] | None = None, error: bool = False) -> None:
self._payload = payload or {}
self._error = error
def raise_for_status(self) -> None:
if self._error:
raise httpx.HTTPStatusError("forbidden", request=None, response=None) # type: ignore[arg-type]
def json(self) -> dict[str, Any]:
return self._payload
class _FakeAsyncClient:
last_post: dict[str, Any] | None = None
last_patch: dict[str, Any] | None = None
post_response: _FakeResponse = _FakeResponse({"id": 42})
patch_response: _FakeResponse = _FakeResponse({})
async def __aenter__(self) -> _FakeAsyncClient:
return self
async def __aexit__(self, exc_type: object, exc: object, tb: object) -> None:
return None
async def post(self, url: str, **kwargs: Any) -> _FakeResponse:
type(self).last_post = {"url": url, **kwargs}
return type(self).post_response
async def patch(self, url: str, **kwargs: Any) -> _FakeResponse:
type(self).last_patch = {"url": url, **kwargs}
return type(self).patch_response
@pytest.fixture(autouse=True)
def _reset_fake_client() -> None:
_FakeAsyncClient.last_post = None
_FakeAsyncClient.last_patch = None
_FakeAsyncClient.post_response = _FakeResponse({"id": 42})
_FakeAsyncClient.patch_response = _FakeResponse({})
async def test_create_review_check_run_posts_in_progress(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(github_checks.httpx, "AsyncClient", _FakeAsyncClient)
check_run_id = await github_checks.create_review_check_run(
owner="acme",
repo="widgets",
head_sha="abc123",
token="tok",
details_url="https://example.com/thread",
)
assert check_run_id == 42
assert _FakeAsyncClient.last_post is not None
assert _FakeAsyncClient.last_post["url"].endswith("/repos/acme/widgets/check-runs")
body = _FakeAsyncClient.last_post["json"]
assert body["name"] == github_checks.REVIEW_CHECK_RUN_NAME
assert body["head_sha"] == "abc123"
assert body["status"] == "in_progress"
assert body["details_url"] == "https://example.com/thread"
async def test_create_review_check_run_returns_none_on_http_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(github_checks.httpx, "AsyncClient", _FakeAsyncClient)
_FakeAsyncClient.post_response = _FakeResponse(error=True)
check_run_id = await github_checks.create_review_check_run(
owner="acme", repo="widgets", head_sha="abc123", token="tok"
)
assert check_run_id is None
async def test_complete_review_check_run_patches_completed(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(github_checks.httpx, "AsyncClient", _FakeAsyncClient)
ok = await github_checks.complete_review_check_run(
owner="acme",
repo="widgets",
check_run_id=42,
token="tok",
conclusion="neutral",
title="Found 2 potential issues",
summary="…",
)
assert ok is True
assert _FakeAsyncClient.last_patch is not None
assert _FakeAsyncClient.last_patch["url"].endswith("/repos/acme/widgets/check-runs/42")
body = _FakeAsyncClient.last_patch["json"]
assert body["status"] == "completed"
assert body["conclusion"] == "neutral"
assert body["output"]["title"] == "Found 2 potential issues"
def test_review_check_conclusion_mapping() -> None:
conclusion, title, _ = github_checks.review_check_conclusion(0)
assert conclusion == "success"
assert title == "No issues found"
conclusion, title, _ = github_checks.review_check_conclusion(1)
assert conclusion == "neutral"
assert "1 potential issue" in title
conclusion, title, _ = github_checks.review_check_conclusion(3)
assert conclusion == "neutral"
assert "3 potential issues" in title
async def test_settle_review_check_run_noop_without_tracked_id(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_get_thread_metadata(thread_id: str) -> dict[str, Any]:
return {}
completed: list[dict[str, Any]] = []
async def fake_complete(**kwargs: Any) -> bool:
completed.append(kwargs)
return True
monkeypatch.setattr(reviewer_publish, "get_thread_metadata", fake_get_thread_metadata)
monkeypatch.setattr(reviewer_publish, "complete_review_check_run", fake_complete)
await reviewer_publish.settle_review_check_run(
thread_id="t1",
owner="acme",
repo="widgets",
token="tok",
conclusion="success",
title="t",
summary="s",
)
assert completed == []
async def test_settle_review_check_run_completes_and_clears(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_get_thread_metadata(thread_id: str) -> dict[str, Any]:
return {"review_check_run_id": 42}
completed: list[dict[str, Any]] = []
metadata_writes: list[dict[str, Any]] = []
async def fake_complete(**kwargs: Any) -> bool:
completed.append(kwargs)
return True
async def fake_set_metadata(thread_id: str, **kwargs: Any) -> None:
metadata_writes.append({"thread_id": thread_id, **kwargs})
monkeypatch.setattr(reviewer_publish, "get_thread_metadata", fake_get_thread_metadata)
monkeypatch.setattr(reviewer_publish, "complete_review_check_run", fake_complete)
monkeypatch.setattr(reviewer_publish, "set_reviewer_thread_metadata", fake_set_metadata)
await reviewer_publish.settle_review_check_run(
thread_id="t1",
owner="acme",
repo="widgets",
token="tok",
conclusion="neutral",
title="t",
summary="s",
)
assert len(completed) == 1
assert completed[0]["check_run_id"] == 42
assert completed[0]["conclusion"] == "neutral"
assert metadata_writes == [
{
"thread_id": "t1",
"extra": {"review_check_run_id": None, "review_check_pending_result": None},
}
]
async def test_settle_review_check_run_keeps_id_on_patch_failure(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_get_thread_metadata(thread_id: str) -> dict[str, Any]:
return {"review_check_run_id": 42}
metadata_writes: list[dict[str, Any]] = []
async def fake_complete(**kwargs: Any) -> bool:
return False
async def fake_set_metadata(thread_id: str, **kwargs: Any) -> None:
metadata_writes.append({"thread_id": thread_id, **kwargs})
monkeypatch.setattr(reviewer_publish, "get_thread_metadata", fake_get_thread_metadata)
monkeypatch.setattr(reviewer_publish, "complete_review_check_run", fake_complete)
monkeypatch.setattr(reviewer_publish, "set_reviewer_thread_metadata", fake_set_metadata)
await reviewer_publish.settle_review_check_run(
thread_id="t1",
owner="acme",
repo="widgets",
token="tok",
conclusion="success",
title="t",
summary="s",
)
assert metadata_writes == [
{
"thread_id": "t1",
"extra": {
"review_check_pending_result": {
"conclusion": "success",
"title": "t",
"summary": "s",
}
},
}
]