diff --git a/agent/reviewer.py b/agent/reviewer.py index cc41a7be..faca30ad 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -4,9 +4,7 @@ Mirrors `agent.server.get_agent`'s sandbox lifecycle but returns a deep agent configured for code review only: narrowed tool set, reviewer-specific system prompt, no commit/push/PR-opening. -Inline review comments are recorded by the agent calling the `github_comment` -tool — one call per distinct issue. The eval harness extracts those calls -from the run's message stream. +Inline review comments are submitted by the agent through the GitHub CLI. """ # ruff: noqa: E402 @@ -39,7 +37,6 @@ from .server import ( ensure_sandbox_for_thread, graph_loaded_for_execution, ) -from .tools import github_comment from .utils.auth import resolve_github_token from .utils.model import ModelKwargs, make_model from .utils.sandbox_paths import aresolve_sandbox_work_dir @@ -70,23 +67,29 @@ You are operating in a remote Linux sandbox at `{working_dir}`. or use `git diff ...`. 4. Read the files the PR changes — and any related files needed to understand the change in context. Use `read_file`, `grep`, `glob`. -5. For each real issue you find, call the `github_comment` tool **once** - with: - - `file`: repo-relative path - - `line`: 1-based line number in the new (post-PR) file - - `body`: a specific description of the issue - - `severity`: one of "Low", "Medium", "High", "Critical" +5. For each real issue you find, submit one inline review comment with + `GH_TOKEN=dummy gh api`: + + `GH_TOKEN=dummy gh api repos///pulls//comments \ + -f body='' \ + -f commit_id='' \ + -f path='' \ + -F line= \ + -f side=RIGHT` + + The `line` value must be a 1-based line number in the new post-PR file + and must be part of the PR diff. If the issue spans multiple lines, anchor + the comment to the most relevant changed line. ### Hard rules - **You are read-only.** Do NOT commit. Do NOT push. Do NOT open or update - PRs. Do NOT post comments via `gh pr comment`. The only way you record - findings is by calling the `github_comment` tool. -- One `github_comment` call per distinct issue. Multiple calls per review + PRs. Do NOT post top-level PR comments via `gh pr comment`. +- One inline `gh api` comment per distinct issue. Multiple comments per review are expected and correct. -- Do not summarize the PR in chat. Do not write a final review essay. - Only `github_comment` calls are scored — anything else is ignored. -- If you find no real issues, make zero `github_comment` calls and stop. +- Do not summarize the PR in chat. Do not write a final review essay. Submit + only inline review comments for real findings. +- If you find no real issues, submit no comments and stop. """ @@ -121,7 +124,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: return create_deep_agent( model=make_model(model_id, **model_kwargs), system_prompt=_reviewer_system_prompt(work_dir), - tools=[github_comment], + tools=[], backend=sandbox_backend, middleware=[ SanitizeToolInputsMiddleware(), diff --git a/agent/tools/__init__.py b/agent/tools/__init__.py index f868c50b..c1857857 100644 --- a/agent/tools/__init__.py +++ b/agent/tools/__init__.py @@ -1,5 +1,4 @@ from .fetch_url import fetch_url -from .github_comment import github_comment from .http_request import http_request from .linear_comment import linear_comment from .linear_create_issue import linear_create_issue @@ -14,7 +13,6 @@ from .web_search import web_search __all__ = [ "fetch_url", - "github_comment", "http_request", "linear_comment", "linear_create_issue", diff --git a/agent/tools/github_comment.py b/agent/tools/github_comment.py deleted file mode 100644 index 1dfbbe1a..00000000 --- a/agent/tools/github_comment.py +++ /dev/null @@ -1,53 +0,0 @@ -from typing import Any, Literal - -Severity = Literal["Low", "Medium", "High", "Critical"] - -_VALID_SEVERITIES: frozenset[str] = frozenset({"Low", "Medium", "High", "Critical"}) - - -def _normalize_severity(value: str) -> Severity: - """Title-case `value` and validate it against the allowed set. - - The model occasionally emits "low"/"HIGH" instead of the title-cased - canonical form. Normalize before recording so we don't burn an LLM - turn on a Pydantic ValidationError retry. - """ - titled = value.strip().title() - if titled not in _VALID_SEVERITIES: - valid = ", ".join(sorted(_VALID_SEVERITIES)) - raise ValueError(f"severity must be one of {valid}; got {value!r}") - return titled # type: ignore[return-value] - - -def github_comment( - file: str, - line: int, - body: str, - severity: str, -) -> dict[str, Any]: - """Record a single inline review comment on the PR under review. - - Call this tool once per issue you find. Multiple calls are expected — one - per distinct concern. The eval harness records every github_comment call - you make and scores them against the PR's golden comments. - - **Do not** use this tool to summarize the PR or make general remarks. Each - call must point at a specific file and line and describe one concrete - issue (bug, security concern, perf problem, correctness issue, etc.). - - Args: - file: Repo-relative path to the file the comment applies to. - line: 1-based line number in the file. - body: The review comment text. Be specific about the issue. - severity: One of "Low", "Medium", "High", "Critical" (case-insensitive). - - Returns: - {"recorded": True, "file", "line", "severity", "body"}. - """ - return { - "recorded": True, - "file": file, - "line": line, - "severity": _normalize_severity(severity), - "body": body, - } diff --git a/agent/webapp.py b/agent/webapp.py index 8a1fd0ec..e84aa674 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -21,6 +21,7 @@ from .utils.auth import ( persist_encrypted_github_token, resolve_github_token_from_email, ) +from .utils.authorship import OPEN_SWE_BOT_NAME from .utils.comments import get_recent_comments from .utils.github_app import get_github_app_installation_token from .utils.github_comments import ( @@ -91,6 +92,16 @@ ALLOWED_GITHUB_ORGS: frozenset[str] = frozenset( for org in os.environ.get("ALLOWED_GITHUB_ORGS", "").split(",") if org.strip() ) +ALLOWED_REVIEWER_GITHUB_ORGS: frozenset[str] = frozenset( + org.strip().lower() + for org in os.environ.get("ALLOWED_REVIEWER_GITHUB_ORGS", "").split(",") + if org.strip() +) +ALLOWED_REVIEWER_GITHUB_REPOS: frozenset[str] = frozenset( + repo.strip().lower() + for repo in os.environ.get("ALLOWED_REVIEWER_GITHUB_REPOS", "").split(",") + if repo.strip() +) LINEAR_API_KEY = os.environ.get("LINEAR_API_KEY", "") @@ -311,6 +322,20 @@ def _is_repo_org_allowed(repo_config: dict[str, str]) -> bool: return owner in ALLOWED_GITHUB_ORGS +def _is_repo_allowed_for_reviewer(repo_config: dict[str, str]) -> bool: + """Check if a repo is allowed for reviewer-agent webhook entrypoints.""" + owner = repo_config.get("owner", "").lower() + name = repo_config.get("name", "").lower() + full_name = f"{owner}/{name}" if owner and name else "" + + if ALLOWED_REVIEWER_GITHUB_REPOS: + return full_name in ALLOWED_REVIEWER_GITHUB_REPOS + + if not ALLOWED_REVIEWER_GITHUB_ORGS: + return True + return owner in ALLOWED_REVIEWER_GITHUB_ORGS + + async def _upsert_slack_thread_repo_metadata( thread_id: str, repo_config: dict[str, str], langgraph_client: LangGraphClient ) -> None: @@ -1100,9 +1125,16 @@ async def health_check() -> dict[str, str]: _SUPPORTED_GH_EVENTS = frozenset( - ["issue_comment", "issues", "pull_request_review_comment", "pull_request_review"] + [ + "issue_comment", + "issues", + "pull_request", + "pull_request_review_comment", + "pull_request_review", + ] ) _SUPPORTED_GH_ISSUE_ACTIONS = frozenset(["edited", "opened", "reopened"]) +_SUPPORTED_GH_PULL_REQUEST_ACTIONS = frozenset(["review_requested"]) def _build_github_issue_comments_text(comments: list[dict[str, Any]]) -> str: @@ -1205,6 +1237,99 @@ async def _trigger_or_queue_run( logger.info("LangGraph run created for thread %s from GitHub PR comment", thread_id) +def _is_open_swe_reviewer_request(payload: dict[str, Any]) -> bool: + reviewer = payload.get("requested_reviewer") or {} + login = reviewer.get("login", "") if isinstance(reviewer, dict) else "" + return login.lower() == OPEN_SWE_BOT_NAME.lower() + + +def build_github_pr_review_prompt( + repo_config: dict[str, str], + pr_number: int, + pr_url: str, + base_sha: str, + head_sha: str, +) -> str: + """Build the user prompt for a reviewer-agent run.""" + return ( + "Please review this GitHub pull request.\n\n" + f"## Repository: {repo_config.get('owner')}/{repo_config.get('name')}\n\n" + f"## Pull Request: {pr_url}\n\n" + f"## PR Number: {pr_number}\n\n" + f"## Base SHA: {base_sha}\n\n" + f"## Head SHA: {head_sha}\n\n" + "Submit findings as inline GitHub review comments. If there are no real issues, " + "submit no comments." + ) + + +async def process_github_pr_review_request(payload: dict[str, Any]) -> None: + """Trigger the reviewer agent when the Open SWE bot is requested on a PR.""" + repo = payload.get("repository", {}) + pull_request = payload.get("pull_request", {}) + repo_config = { + "owner": repo.get("owner", {}).get("login", ""), + "name": repo.get("name", ""), + } + pr_number = pull_request.get("number") + pr_url = pull_request.get("html_url", "") or pull_request.get("url", "") + branch_name = pull_request.get("head", {}).get("ref", "") + base_sha = pull_request.get("base", {}).get("sha", "") + head_sha = pull_request.get("head", {}).get("sha", "") + github_login = payload.get("sender", {}).get("login", "") + github_user_id = payload.get("sender", {}).get("id") + + if not pr_number or not pr_url or not base_sha or not head_sha: + logger.warning("Missing PR review request context, skipping reviewer run") + return + + owner = repo_config.get("owner", "") + name = repo_config.get("name", "") + stable_key = f"{owner}/{name}/pr/{pr_number}/reviewer" + thread_id = str(uuid.uuid5(uuid.NAMESPACE_URL, stable_key)) + + app_token = await get_github_app_installation_token() + if not app_token: + logger.warning("No GitHub App token available for PR reviewer request") + return + + try: + await persist_encrypted_github_token(thread_id, app_token) + except Exception: + logger.warning("Could not persist bot token for reviewer thread %s", thread_id) + return + + prompt = build_github_pr_review_prompt(repo_config, pr_number, pr_url, base_sha, head_sha) + configurable: dict[str, Any] = { + "source": "github", + "github_login": github_login, + "github_user_id": github_user_id, + "repo": repo_config, + "pr_number": pr_number, + "review_requested": True, + } + + if branch_name: + configurable["branch_name"] = branch_name + + thread_active = await is_thread_active(thread_id) + if thread_active: + logger.info("Reviewer thread %s is busy, queuing PR review request", thread_id) + await queue_message_for_thread(thread_id, prompt) + return + + logger.info("Creating reviewer run for thread %s from GitHub PR review request", thread_id) + langgraph_client = get_client(url=LANGGRAPH_URL) + await langgraph_client.runs.create( + thread_id, + "reviewer", + input={"messages": [{"role": "user", "content": prompt}]}, + config={"configurable": configurable, "metadata": _AGENT_VERSION_METADATA}, + if_not_exists="create", + ) + logger.info("Reviewer run created for thread %s from GitHub PR review request", thread_id) + + async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str | None: """Resolve and persist a GitHub token for a thread when available. @@ -1473,12 +1598,45 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> logger.exception("Failed to parse GitHub webhook JSON") return {"status": "error", "message": "Invalid JSON"} - # Check org allowlist webhook_repo = payload.get("repository", {}) webhook_repo_config = { "owner": webhook_repo.get("owner", {}).get("login", ""), "name": webhook_repo.get("name", ""), } + + issue = payload.get("issue", {}) + is_pull_request_comment = bool(event_type == "issue_comment" and issue.get("pull_request")) + is_issue_comment = bool(event_type == "issue_comment" and not issue.get("pull_request")) + is_issue_event = event_type == "issues" + is_pull_request_event = event_type == "pull_request" + + if is_pull_request_event: + action = payload.get("action", "") + if action not in _SUPPORTED_GH_PULL_REQUEST_ACTIONS: + logger.info("Ignoring unsupported GitHub pull_request action: %s", action) + return { + "status": "ignored", + "reason": f"Unsupported GitHub pull_request action: {action}", + } + if not _is_open_swe_reviewer_request(payload): + logger.info("Ignoring PR review request for a different reviewer") + return {"status": "ignored", "reason": "Review request is not for open-swe bot"} + if not _is_repo_allowed_for_reviewer(webhook_repo_config): + logger.warning( + "Rejecting GitHub reviewer webhook: repo '%s/%s' failed reviewer allowlist", + webhook_repo_config.get("owner"), + webhook_repo_config.get("name"), + ) + if ALLOWED_REVIEWER_GITHUB_REPOS: + reason = "Repository not in allowlist" + else: + reason = "Repository org not in allowlist" + return {"status": "ignored", "reason": reason} + + logger.info("Accepted GitHub PR review request webhook, scheduling reviewer task") + background_tasks.add_task(process_github_pr_review_request, payload) + return {"status": "accepted", "message": "Processing GitHub PR review request"} + if not _is_repo_org_allowed(webhook_repo_config): logger.warning( "Rejecting GitHub webhook: org '%s' not in ALLOWED_GITHUB_ORGS", @@ -1486,11 +1644,6 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> ) return {"status": "ignored", "reason": "Repository org not in allowlist"} - issue = payload.get("issue", {}) - is_pull_request_comment = bool(event_type == "issue_comment" and issue.get("pull_request")) - is_issue_comment = bool(event_type == "issue_comment" and not issue.get("pull_request")) - is_issue_event = event_type == "issues" - if is_issue_event: action = payload.get("action", "") if action not in _SUPPORTED_GH_ISSUE_ACTIONS: diff --git a/tests/test_github_issue_webhook.py b/tests/test_github_issue_webhook.py index ec6e9471..d92bc6fc 100644 --- a/tests/test_github_issue_webhook.py +++ b/tests/test_github_issue_webhook.py @@ -66,6 +66,39 @@ def test_build_github_issue_followup_prompt_only_includes_comment() -> None: assert "## Title" not in prompt +def test_reviewer_repo_allowlist_allows_matching_repo(monkeypatch) -> None: + monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_ORGS", frozenset()) + monkeypatch.setattr( + webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) + ) + + assert ( + webapp._is_repo_allowed_for_reviewer({"owner": "langchain-ai", "name": "open-swe"}) is True + ) + + +def test_reviewer_repo_allowlist_blocks_non_matching_repo(monkeypatch) -> None: + monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_ORGS", frozenset({"langchain-ai"})) + monkeypatch.setattr( + webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) + ) + + assert ( + webapp._is_repo_allowed_for_reviewer({"owner": "langchain-ai", "name": "public-demo"}) + is False + ) + + +def test_reviewer_org_allowlist_allows_all_repos_in_org(monkeypatch) -> None: + monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_ORGS", frozenset({"langchain-ai"})) + monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset()) + + assert ( + webapp._is_repo_allowed_for_reviewer({"owner": "langchain-ai", "name": "any-repo"}) is True + ) + assert webapp._is_repo_allowed_for_reviewer({"owner": "other-org", "name": "any-repo"}) is False + + def test_github_webhook_accepts_issue_events(monkeypatch) -> None: called: dict[str, object] = {} @@ -158,6 +191,183 @@ def test_github_webhook_accepts_issue_comment_events(monkeypatch) -> None: assert called["event_type"] == "issue_comment" +def test_github_webhook_blocks_reviewer_repo_not_in_reviewer_repo_allowlist(monkeypatch) -> None: + called = False + + async def fake_process_github_pr_review_request(payload: dict[str, object]) -> None: + nonlocal called + called = True + + monkeypatch.setattr( + webapp, "process_github_pr_review_request", fake_process_github_pr_review_request + ) + monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) + monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_ORGS", frozenset({"langchain-ai"})) + monkeypatch.setattr( + webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) + ) + + client = TestClient(webapp.app) + response = _post_github_webhook( + client, + "pull_request", + { + "action": "review_requested", + "requested_reviewer": {"login": "open-swe[bot]"}, + "pull_request": { + "number": 1244, + "html_url": "https://github.com/langchain-ai/public-demo/pull/1244", + "base": {"sha": "base-sha"}, + "head": {"sha": "head-sha", "ref": "feature-branch"}, + }, + "repository": {"owner": {"login": "langchain-ai"}, "name": "public-demo"}, + "sender": {"login": "octocat"}, + }, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "ignored", "reason": "Repository not in allowlist"} + assert called is False + + +def test_github_webhook_accepts_open_swe_review_requested(monkeypatch) -> None: + called: dict[str, object] = {} + + async def fake_process_github_pr_review_request(payload: dict[str, object]) -> None: + called["payload"] = payload + + monkeypatch.setattr( + webapp, "process_github_pr_review_request", fake_process_github_pr_review_request + ) + monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) + monkeypatch.setattr( + webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) + ) + + client = TestClient(webapp.app) + response = _post_github_webhook( + client, + "pull_request", + { + "action": "review_requested", + "requested_reviewer": {"login": "open-swe[bot]"}, + "pull_request": { + "number": 1244, + "html_url": "https://github.com/langchain-ai/open-swe/pull/1244", + "base": {"sha": "base-sha"}, + "head": {"sha": "head-sha", "ref": "feature-branch"}, + }, + "repository": {"owner": {"login": "langchain-ai"}, "name": "open-swe"}, + "sender": {"login": "octocat"}, + }, + ) + + assert response.status_code == 200 + assert response.json()["status"] == "accepted" + assert called["payload"]["requested_reviewer"]["login"] == "open-swe[bot]" + + +def test_github_webhook_ignores_review_requested_for_other_reviewer(monkeypatch) -> None: + called = False + + async def fake_process_github_pr_review_request(payload: dict[str, object]) -> None: + nonlocal called + called = True + + monkeypatch.setattr( + webapp, "process_github_pr_review_request", fake_process_github_pr_review_request + ) + monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) + + client = TestClient(webapp.app) + response = _post_github_webhook( + client, + "pull_request", + { + "action": "review_requested", + "requested_reviewer": {"login": "someone-else"}, + "pull_request": { + "number": 1244, + "html_url": "https://github.com/langchain-ai/open-swe/pull/1244", + "base": {"sha": "base-sha"}, + "head": {"sha": "head-sha", "ref": "feature-branch"}, + }, + "repository": {"owner": {"login": "langchain-ai"}, "name": "open-swe"}, + "sender": {"login": "octocat"}, + }, + ) + + assert response.status_code == 200 + assert response.json()["status"] == "ignored" + assert called is False + + +def test_process_github_pr_review_request_creates_reviewer_run(monkeypatch) -> None: + captured: dict[str, object] = {} + + async def fake_get_github_app_installation_token() -> str | None: + return "app-token" + + async def fake_persist_encrypted_github_token(thread_id: str, token: str) -> str: + captured["persist_thread_id"] = thread_id + captured["persist_token"] = token + return "encrypted-token" + + async def fake_is_thread_active(thread_id: str) -> bool: + captured["active_thread_id"] = thread_id + return False + + class _FakeRunsClient: + async def create(self, thread_id: str, graph: str, **kwargs) -> None: + captured["thread_id"] = thread_id + captured["graph"] = graph + captured["kwargs"] = kwargs + + class _FakeLangGraphClient: + runs = _FakeRunsClient() + + monkeypatch.setattr( + webapp, "get_github_app_installation_token", fake_get_github_app_installation_token + ) + monkeypatch.setattr( + webapp, "persist_encrypted_github_token", fake_persist_encrypted_github_token + ) + monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active) + monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClient()) + + asyncio.run( + webapp.process_github_pr_review_request( + { + "action": "review_requested", + "requested_reviewer": {"login": "open-swe[bot]"}, + "pull_request": { + "number": 1244, + "html_url": "https://github.com/langchain-ai/open-swe/pull/1244", + "base": {"sha": "base-sha"}, + "head": {"sha": "head-sha", "ref": "feature-branch"}, + }, + "repository": {"owner": {"login": "langchain-ai"}, "name": "open-swe"}, + "sender": {"login": "octocat", "id": 123}, + } + ) + ) + + kwargs = captured["kwargs"] + prompt = kwargs["input"]["messages"][0]["content"] + config = kwargs["config"]["configurable"] + + assert captured["graph"] == "reviewer" + assert captured["persist_token"] == "app-token" + assert captured["persist_thread_id"] == captured["thread_id"] + assert "https://github.com/langchain-ai/open-swe/pull/1244" in prompt + assert "Base SHA: base-sha" in prompt + assert "Head SHA: head-sha" in prompt + assert config["source"] == "github" + assert config["repo"] == {"owner": "langchain-ai", "name": "open-swe"} + assert config["pr_number"] == 1244 + assert config["review_requested"] is True + + def test_process_github_issue_uses_resolved_user_token_for_reaction(monkeypatch) -> None: captured: dict[str, object] = {}