diff --git a/agent/webhooks/github.py b/agent/webhooks/github.py index 8d076bf3..4258a092 100644 --- a/agent/webhooks/github.py +++ b/agent/webhooks/github.py @@ -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\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: diff --git a/agent/webhooks/github_routes.py b/agent/webhooks/github_routes.py index a38a93c4..1df20311 100644 --- a/agent/webhooks/github_routes.py +++ b/agent/webhooks/github_routes.py @@ -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 diff --git a/tests/dashboard/test_public_repo_org_gate.py b/tests/dashboard/test_public_repo_org_gate.py index f41d1be3..fcfe3d81 100644 --- a/tests/dashboard/test_public_repo_org_gate.py +++ b/tests/dashboard/test_public_repo_org_gate.py @@ -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: diff --git a/tests/github/test_github_issue_webhook.py b/tests/github/test_github_issue_webhook.py index 84a62d1a..54021ae1 100644 --- a/tests/github/test_github_issue_webhook.py +++ b/tests/github/test_github_issue_webhook.py @@ -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"] == "" diff --git a/tests/github/test_review_comment_command.py b/tests/github/test_review_comment_command.py new file mode 100644 index 00000000..81580200 --- /dev/null +++ b/tests/github/test_review_comment_command.py @@ -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 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", + )