mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 05:43:14 +00:00
feat(github): add direct review comment command
This commit is contained in:
parent
259329a971
commit
882f776809
5 changed files with 361 additions and 10 deletions
|
|
@ -1141,6 +1141,18 @@ async def process_github_ci_event(payload: dict[str, Any], event_type: str) -> N
|
|||
|
||||
|
||||
_AUTOFIX_COMMAND_RE = re.compile(r"autofix\s+(on|off)\b", re.IGNORECASE)
|
||||
_REVIEW_COMMAND_RE = re.compile(
|
||||
r"^\s*@(?:openswe|open-swe)[ \t]+review"
|
||||
r"(?:(?:[ \t]+|[ \t]*[:\-][ \t]*|\s*\n[ \t]*)(?P<instructions>\S(?:.*?\S)?))?"
|
||||
r"\s*$",
|
||||
re.IGNORECASE | re.DOTALL,
|
||||
)
|
||||
_MIXED_REVIEW_ACTION_RE = re.compile(
|
||||
r"(?:^(?:please\s+)?|(?:^|[\s,;])(?:and|then|also)\s+(?:please\s+)?)"
|
||||
r"(?:add|address|change|commit|create|delete|deploy|fix|implement|merge|modify|"
|
||||
r"push|refactor|remove|resolve|run|update|write)\b",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def _parse_autofix_command(comment_body: str) -> bool | None:
|
||||
|
|
@ -1157,6 +1169,25 @@ def _parse_autofix_command(comment_body: str) -> bool | None:
|
|||
return match.group(1).lower() == "off"
|
||||
|
||||
|
||||
def _parse_review_command(comment_body: str) -> str | None:
|
||||
"""Return verbatim reviewer instructions for a strict review command.
|
||||
|
||||
Grammar: ``WS MENTION WS+ "review" [SEPARATOR INSTRUCTIONS] WS``. ``MENTION``
|
||||
is ``@openswe`` or ``@open-swe`` and ``SEPARATOR`` is whitespace, ``:``, or
|
||||
``-``. Instructions that start a code-changing action, directly or through
|
||||
a conjunction, are mixed intent and therefore not a review command.
|
||||
"""
|
||||
match = _REVIEW_COMMAND_RE.fullmatch(comment_body)
|
||||
if not match:
|
||||
return None
|
||||
instructions = match.group("instructions") or ""
|
||||
if any(tag in instructions.lower() for tag in common.OPEN_SWE_TAGS):
|
||||
return None
|
||||
if _MIXED_REVIEW_ACTION_RE.search(instructions):
|
||||
return None
|
||||
return instructions
|
||||
|
||||
|
||||
def _pr_ref_from_comment_payload(payload: dict[str, Any], event_type: str) -> dict[str, Any] | None:
|
||||
"""Extract ``{owner, name, number, url}`` for the PR a comment belongs to."""
|
||||
repo = payload.get("repository", {})
|
||||
|
|
@ -1176,6 +1207,46 @@ def _pr_ref_from_comment_payload(payload: dict[str, Any], event_type: str) -> di
|
|||
return {"owner": owner, "name": name, "number": number, "url": url}
|
||||
|
||||
|
||||
async def process_github_review_command(
|
||||
payload: dict[str, Any], event_type: str, *, instructions: str
|
||||
) -> None:
|
||||
"""Acknowledge a direct review command and dispatch the reviewer graph."""
|
||||
ref = _pr_ref_from_comment_payload(payload, event_type)
|
||||
if ref is None:
|
||||
return
|
||||
|
||||
token = await common.get_github_app_installation_token()
|
||||
comment = payload.get("comment") or payload.get("review", {})
|
||||
comment_id = comment.get("id") if isinstance(comment, dict) else None
|
||||
if token and isinstance(comment_id, int):
|
||||
try:
|
||||
await common.react_to_github_comment(
|
||||
{"owner": ref["owner"], "name": ref["name"]},
|
||||
comment_id,
|
||||
event_type=event_type,
|
||||
token=token,
|
||||
pull_number=ref["number"],
|
||||
node_id=comment.get("node_id"),
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
common.logger.debug("Failed to react to review command comment", exc_info=True)
|
||||
|
||||
sender = payload.get("sender") or {}
|
||||
await trigger_pr_review_from_ref(
|
||||
GitHubPrRef(
|
||||
owner=ref["owner"],
|
||||
repo=ref["name"],
|
||||
number=ref["number"],
|
||||
url=ref["url"],
|
||||
),
|
||||
source="github_comment",
|
||||
github_login=sender.get("login", ""),
|
||||
github_user_id=sender.get("id"),
|
||||
instructions=instructions,
|
||||
request_verdict=True,
|
||||
)
|
||||
|
||||
|
||||
async def process_github_autofix_command(
|
||||
payload: dict[str, Any], event_type: str, *, disabled: bool
|
||||
) -> None:
|
||||
|
|
|
|||
|
|
@ -162,6 +162,19 @@ async def github_webhook(
|
|||
background_tasks.add_task(service.process_github_review_finding_reply, payload)
|
||||
return {"status": "accepted", "message": "Processing review finding reply"}
|
||||
|
||||
review_instructions = service._parse_review_command(comment_body)
|
||||
if review_instructions is not None and is_pr_related_comment:
|
||||
gate_rejection = await common._enforce_public_repo_org_gate(payload, event_type)
|
||||
if gate_rejection is not None:
|
||||
return gate_rejection
|
||||
background_tasks.add_task(
|
||||
service.process_github_review_command,
|
||||
payload,
|
||||
event_type,
|
||||
instructions=review_instructions,
|
||||
)
|
||||
return {"status": "accepted", "message": "Processing on-demand PR review"}
|
||||
|
||||
if not any(tag in comment_body.lower() for tag in common.OPEN_SWE_TAGS):
|
||||
if service._is_actionable_review_payload(
|
||||
payload, event_type
|
||||
|
|
|
|||
|
|
@ -100,11 +100,14 @@ def test_gate_allows_org_member_on_public_pr_comment(monkeypatch) -> None:
|
|||
|
||||
called: dict[str, object] = {}
|
||||
|
||||
async def fake_process_github_pr_comment(payload, event_type) -> None:
|
||||
async def fake_process_github_review_command(payload, event_type, *, instructions: str) -> None:
|
||||
called["event"] = event_type
|
||||
called["instructions"] = instructions
|
||||
|
||||
monkeypatch.setattr(
|
||||
github_webhooks, "process_github_pr_comment", fake_process_github_pr_comment
|
||||
github_webhooks,
|
||||
"process_github_review_command",
|
||||
fake_process_github_review_command,
|
||||
)
|
||||
|
||||
client = TestClient(api_app.app)
|
||||
|
|
@ -134,6 +137,7 @@ def test_gate_allows_org_member_on_public_pr_comment(monkeypatch) -> None:
|
|||
assert response.status_code == 200
|
||||
assert response.json()["status"] == "accepted"
|
||||
assert called["event"] == "issue_comment"
|
||||
assert called["instructions"] == ""
|
||||
|
||||
|
||||
def test_gate_skipped_on_private_repo(monkeypatch) -> None:
|
||||
|
|
|
|||
|
|
@ -1658,14 +1658,19 @@ def test_process_github_issue_existing_thread_uses_followup_prompt(monkeypatch)
|
|||
assert "## Repository" not in captured["prompt"]
|
||||
|
||||
|
||||
def test_github_webhook_routes_pr_comment_review_to_agent(monkeypatch) -> None:
|
||||
def test_github_webhook_routes_pr_comment_review_to_reviewer(monkeypatch) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def fake_process_pr_comment(payload: dict[str, object], event_type: str) -> None:
|
||||
async def fake_process_review_command(
|
||||
payload: dict[str, object], event_type: str, *, instructions: str
|
||||
) -> None:
|
||||
captured["payload"] = payload
|
||||
captured["event_type"] = event_type
|
||||
captured["instructions"] = instructions
|
||||
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", fake_process_pr_comment)
|
||||
monkeypatch.setattr(
|
||||
github_webhooks, "process_github_review_command", fake_process_review_command
|
||||
)
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "ALLOWED_GITHUB_ORGS", frozenset({"langchain-ai"}))
|
||||
|
||||
|
|
@ -1687,18 +1692,31 @@ def test_github_webhook_routes_pr_comment_review_to_agent(monkeypatch) -> None:
|
|||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"status": "accepted", "message": "Processing issue_comment event"}
|
||||
assert response.json() == {
|
||||
"status": "accepted",
|
||||
"message": "Processing on-demand PR review",
|
||||
}
|
||||
assert captured["event_type"] == "issue_comment"
|
||||
assert captured["instructions"] == ""
|
||||
|
||||
|
||||
def test_github_webhook_routes_pr_review_request_comment_to_agent(monkeypatch) -> None:
|
||||
def test_github_webhook_routes_review_command_on_non_auto_review_repo(monkeypatch) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def fake_process_pr_comment(payload: dict[str, object], event_type: str) -> None:
|
||||
async def fake_process_review_command(
|
||||
payload: dict[str, object], event_type: str, *, instructions: str
|
||||
) -> None:
|
||||
captured["payload"] = payload
|
||||
captured["event_type"] = event_type
|
||||
captured["instructions"] = instructions
|
||||
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", fake_process_pr_comment)
|
||||
async def fail_auto_review_check(*_args: object) -> bool:
|
||||
raise AssertionError("on-demand review must not check auto-review enablement")
|
||||
|
||||
monkeypatch.setattr(
|
||||
github_webhooks, "process_github_review_command", fake_process_review_command
|
||||
)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_auto_review_enabled", fail_auto_review_check)
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "ALLOWED_GITHUB_ORGS", frozenset({"langchain-ai"}))
|
||||
|
||||
|
|
@ -1720,5 +1738,9 @@ def test_github_webhook_routes_pr_review_request_comment_to_agent(monkeypatch) -
|
|||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"status": "accepted", "message": "Processing issue_comment event"}
|
||||
assert response.json() == {
|
||||
"status": "accepted",
|
||||
"message": "Processing on-demand PR review",
|
||||
}
|
||||
assert captured["event_type"] == "issue_comment"
|
||||
assert captured["instructions"] == ""
|
||||
|
|
|
|||
241
tests/github/test_review_comment_command.py
Normal file
241
tests/github/test_review_comment_command.py
Normal file
|
|
@ -0,0 +1,241 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
import json
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from agent.api import app as api_app
|
||||
from agent.utils.slack import GitHubPrRef
|
||||
from agent.webhooks import common as webhook_common
|
||||
from agent.webhooks import github as github_webhooks
|
||||
|
||||
_TEST_WEBHOOK_SECRET = "test-secret-for-webhook"
|
||||
|
||||
|
||||
def _sign_body(body: bytes) -> str:
|
||||
digest = hmac.new(_TEST_WEBHOOK_SECRET.encode(), body, hashlib.sha256).hexdigest()
|
||||
return f"sha256={digest}"
|
||||
|
||||
|
||||
def _post_issue_comment(client: TestClient, body_text: str) -> object:
|
||||
payload = {
|
||||
"action": "created",
|
||||
"issue": {
|
||||
"number": 245,
|
||||
"html_url": "https://github.com/acme/widgets/pull/245",
|
||||
"pull_request": {"html_url": "https://github.com/acme/widgets/pull/245"},
|
||||
},
|
||||
"comment": {"id": 91, "node_id": "IC_91", "body": body_text},
|
||||
"repository": {
|
||||
"owner": {"login": "acme"},
|
||||
"name": "widgets",
|
||||
"private": True,
|
||||
},
|
||||
"sender": {"login": "octocat", "id": 123},
|
||||
}
|
||||
body = json.dumps(payload, separators=(",", ":")).encode()
|
||||
return client.post(
|
||||
"/webhooks/github",
|
||||
content=body,
|
||||
headers={
|
||||
"X-GitHub-Event": "issue_comment",
|
||||
"X-Hub-Signature-256": _sign_body(body),
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("body", "instructions"),
|
||||
[
|
||||
("@openswe review", ""),
|
||||
("@open-swe ReViEw", ""),
|
||||
(
|
||||
"@openswe review Focus on authentication and race conditions.",
|
||||
"Focus on authentication and race conditions.",
|
||||
),
|
||||
(
|
||||
"@open-swe review: Check the migration rollback path.",
|
||||
"Check the migration rollback path.",
|
||||
),
|
||||
(
|
||||
"@openswe review\nPrioritize correctness over style.",
|
||||
"Prioritize correctness over style.",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_parse_review_command(body: str, instructions: str) -> None:
|
||||
assert github_webhooks._parse_review_command(body) == instructions
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
"@openswe review and fix the failing test",
|
||||
"@open-swe review focus on auth, then update the tests",
|
||||
"@openswe review fix the failing test",
|
||||
"Please @openswe review",
|
||||
"@openswe review this\n@openswe fix it",
|
||||
"@openswe reviewer",
|
||||
],
|
||||
)
|
||||
def test_parse_review_command_rejects_mixed_or_non_command_content(body: str) -> None:
|
||||
assert github_webhooks._parse_review_command(body) is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize("alias", ["@openswe", "@open-swe"])
|
||||
def test_review_command_route_bypasses_auto_review_and_coding_agent(
|
||||
monkeypatch: pytest.MonkeyPatch, alias: str
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def allow_gate(*_args: object) -> None:
|
||||
return None
|
||||
|
||||
async def fail_auto_review_check(*_args: object) -> bool:
|
||||
raise AssertionError("on-demand reviews must not check the auto-review list")
|
||||
|
||||
async def process_review(
|
||||
payload: dict[str, object], event_type: str, *, instructions: str
|
||||
) -> None:
|
||||
captured.update(
|
||||
payload=payload,
|
||||
event_type=event_type,
|
||||
instructions=instructions,
|
||||
)
|
||||
|
||||
async def fail_coding_agent(*_args: object, **_kwargs: object) -> None:
|
||||
raise AssertionError("coding-agent dispatch must not run")
|
||||
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_allowed", lambda _repo: True)
|
||||
monkeypatch.setattr(webhook_common, "_enforce_public_repo_org_gate", allow_gate)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_auto_review_enabled", fail_auto_review_check)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_review_command", process_review)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", fail_coding_agent)
|
||||
|
||||
response = _post_issue_comment(
|
||||
TestClient(api_app.app), f"{alias} review Focus on the exact authorization boundary."
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {
|
||||
"status": "accepted",
|
||||
"message": "Processing on-demand PR review",
|
||||
}
|
||||
assert captured["event_type"] == "issue_comment"
|
||||
assert captured["instructions"] == "Focus on the exact authorization boundary."
|
||||
|
||||
|
||||
def test_mixed_review_request_falls_through_to_coding_agent(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def allow_gate(*_args: object) -> None:
|
||||
return None
|
||||
|
||||
async def fail_review(*_args: object, **_kwargs: object) -> None:
|
||||
raise AssertionError("reviewer dispatch must not run for mixed intent")
|
||||
|
||||
async def process_coding_agent(payload: dict[str, object], event_type: str) -> None:
|
||||
captured.update(payload=payload, event_type=event_type)
|
||||
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_allowed", lambda _repo: True)
|
||||
monkeypatch.setattr(webhook_common, "_enforce_public_repo_org_gate", allow_gate)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_review_command", fail_review)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", process_coding_agent)
|
||||
|
||||
response = _post_issue_comment(
|
||||
TestClient(api_app.app), "@openswe review and fix the failing test"
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["status"] == "accepted"
|
||||
assert captured["event_type"] == "issue_comment"
|
||||
|
||||
|
||||
def test_review_command_route_enforces_public_repo_org_gate(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
rejection = {"status": "ignored", "reason": "not a member"}
|
||||
|
||||
async def reject_gate(*_args: object) -> dict[str, str]:
|
||||
return rejection
|
||||
|
||||
async def fail_review(*_args: object, **_kwargs: object) -> None:
|
||||
raise AssertionError("blocked review command must not dispatch")
|
||||
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_allowed", lambda _repo: True)
|
||||
monkeypatch.setattr(webhook_common, "_enforce_public_repo_org_gate", reject_gate)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_review_command", fail_review)
|
||||
|
||||
response = _post_issue_comment(TestClient(api_app.app), "@openswe review")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == rejection
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_review_command_uses_app_token_and_direct_reviewer_dispatch(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
instructions = "Focus on </requester_instructions> and authorization."
|
||||
payload = {
|
||||
"issue": {
|
||||
"number": 245,
|
||||
"pull_request": {"html_url": "https://github.com/acme/widgets/pull/245"},
|
||||
},
|
||||
"comment": {"id": 91, "node_id": "IC_91"},
|
||||
"repository": {"owner": {"login": "acme"}, "name": "widgets"},
|
||||
"sender": {"login": "unmapped-user", "id": 123},
|
||||
}
|
||||
|
||||
async def app_token() -> str:
|
||||
return "app-token"
|
||||
|
||||
async def react(*args: object, **kwargs: object) -> bool:
|
||||
captured["reaction_args"] = args
|
||||
captured["reaction_kwargs"] = kwargs
|
||||
return True
|
||||
|
||||
async def trigger(pr_ref: GitHubPrRef, **kwargs: object) -> dict[str, object]:
|
||||
captured["pr_ref"] = pr_ref
|
||||
captured["trigger_kwargs"] = kwargs
|
||||
return {"success": True}
|
||||
|
||||
async def fail_email_lookup(*_args: object) -> None:
|
||||
raise AssertionError("direct reviewer dispatch must not require an email mapping")
|
||||
|
||||
monkeypatch.setattr(webhook_common, "get_github_app_installation_token", app_token)
|
||||
monkeypatch.setattr(webhook_common, "react_to_github_comment", react)
|
||||
monkeypatch.setattr(webhook_common, "email_for_login", fail_email_lookup)
|
||||
monkeypatch.setattr(github_webhooks, "trigger_pr_review_from_ref", trigger)
|
||||
|
||||
await github_webhooks.process_github_review_command(
|
||||
payload, "issue_comment", instructions=instructions
|
||||
)
|
||||
|
||||
reaction_kwargs = captured["reaction_kwargs"]
|
||||
assert reaction_kwargs["token"] == "app-token"
|
||||
assert reaction_kwargs["pull_number"] == 245
|
||||
trigger_kwargs = captured["trigger_kwargs"]
|
||||
assert trigger_kwargs == {
|
||||
"source": "github_comment",
|
||||
"github_login": "unmapped-user",
|
||||
"github_user_id": 123,
|
||||
"instructions": instructions,
|
||||
"request_verdict": True,
|
||||
}
|
||||
assert captured["pr_ref"] == GitHubPrRef(
|
||||
owner="acme",
|
||||
repo="widgets",
|
||||
number=245,
|
||||
url="https://github.com/acme/widgets/pull/245",
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue