diff --git a/agent/prompt.py b/agent/prompt.py index 7ccce69b..66c80be3 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -126,7 +126,7 @@ If you make changes, communicate updates in the source channel: - For GitHub-triggered tasks, use `GH_TOKEN=dummy gh issue comment` or `GH_TOKEN=dummy gh pr comment` only after confirming the target issue or pull request. - If the task was not triggered from a known source (no Slack thread, no Linear ticket, no GitHub issue), skip the notification step. -If a Slack-triggered request is asking you to review a GitHub pull request, do not clone the repo, edit files, commit, push, or open a PR. Call `request_pr_review` once with the GitHub PR URL, then use `slack_thread_reply` to say whether the review was started or why it could not be started, and stop. +If a Slack- or GitHub-triggered request is asking you to review a GitHub pull request, do not clone the repo, edit files, commit, push, or open a PR. Call `request_pr_review` once with the GitHub PR URL, then reply in the source channel to say whether the review was started or why it could not be started, and stop. First decide whether the user is asking for code/repository changes or for information only. Do not create commits, branches, or pull requests for questions, explanations, status checks, or other requests that can be fully answered without changing files. diff --git a/agent/tools/request_pr_review.py b/agent/tools/request_pr_review.py index d200009c..f255e1bb 100644 --- a/agent/tools/request_pr_review.py +++ b/agent/tools/request_pr_review.py @@ -1,6 +1,8 @@ import asyncio from typing import Any +from langgraph.config import get_config + from agent.utils.slack import parse_github_pr_url from agent.webapp import trigger_pr_review_from_ref @@ -14,4 +16,16 @@ def request_pr_review(pr_url: str) -> dict[str, Any]: "error": "Expected a GitHub PR URL like https://github.com/OWNER/REPO/pull/NUMBER", } - return asyncio.run(trigger_pr_review_from_ref(pr_ref, source="slack")) + configurable = get_config().get("configurable", {}) + source = configurable.get("source") or "agent" + slack_thread = configurable.get("slack_thread") or {} + return asyncio.run( + trigger_pr_review_from_ref( + pr_ref, + source=source, + github_login=configurable.get("github_login", ""), + github_user_id=configurable.get("github_user_id"), + slack_channel_id=slack_thread.get("channel_id", ""), + slack_thread_ts=slack_thread.get("thread_ts", ""), + ) + ) diff --git a/agent/utils/slack.py b/agent/utils/slack.py index c0ee4595..113c0a48 100644 --- a/agent/utils/slack.py +++ b/agent/utils/slack.py @@ -690,12 +690,12 @@ TRACE_REPLY_TIPS: tuple[str, ...] = ( "I can spawn subagents for independent subtasks — useful for parallel research or fan-out work.", "Click `View trace` above to watch every tool call and model response live in LangSmith.", "React to my final reply with :+1: or :-1: to share feedback — it helps me get better.", - "Send me `review ` in Slack and I'll spin up the reviewer agent to leave inline comments on that PR.", + "Ask me to review a GitHub PR in Slack and I'll spin up the reviewer agent to leave inline comments.", "Pasting a GitHub URL into your message also works to point me at a repo — no `repo:` prefix needed.", "Attach screenshots or images directly in Slack or Linear — I'll read them as part of the task context.", "Tag `@openswe` on a Linear issue and I'll pull in the full title, description, and comment thread before starting.", "I also pick up `@openswe` mentions in GitHub issue bodies and comments — not just on PRs.", - "On a GitHub PR, comment `@open-swe review` to trigger the reviewer agent and get inline review comments.", + "On a GitHub PR, ask me to review it and I'll hand it off to the reviewer agent for inline comments.", "Each thread keeps a persistent sandbox — follow-up runs reuse the same workspace, so my context sticks around.", "Paste a Slack message link from another thread and I'll fetch its content (and any images) as extra context.", "Ask me to search the web — I have a `web_search` tool for finding docs, examples, and GitHub repos mid-task.", diff --git a/agent/webapp.py b/agent/webapp.py index 73043112..43203d1f 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -71,9 +71,7 @@ from .utils.slack import ( format_slack_messages_for_prompt, get_slack_user_info, get_slack_user_names, - looks_like_slack_pr_review_command, parse_github_pr_url, - parse_slack_review_command, post_slack_thread_reply, post_slack_trace_reply, resolve_slack_links_in_context, @@ -1283,23 +1281,6 @@ async def slack_webhook(request: Request, background_tasks: BackgroundTasks) -> if bot_user_id and user_id == bot_user_id: return {"status": "ignored", "reason": "Event from this bot user"} - clean_text = strip_bot_mention(text, bot_user_id, bot_username=SLACK_BOT_USERNAME) - pr_ref = parse_slack_review_command(clean_text) - if pr_ref: - if not await _is_repo_enabled_for_review({"owner": pr_ref.owner, "name": pr_ref.repo}): - return {"status": "ignored", "reason": "Repository not enabled for review"} - background_tasks.add_task(process_slack_pr_review_request, pr_ref, channel_id, thread_ts) - return {"status": "accepted", "message": "Slack PR review request queued"} - - if looks_like_slack_pr_review_command(clean_text): - background_tasks.add_task( - post_slack_thread_reply, - channel_id, - thread_ts, - "To request a PR review, use `@open-swe review https://github.com/OWNER/REPO/pull/NUMBER`.", - ) - return {"status": "ignored", "reason": "Malformed Slack PR review command"} - event_data = { "channel_id": channel_id, "thread_ts": thread_ts, @@ -2283,12 +2264,24 @@ async def process_github_pr_comment(payload: dict[str, Any], event_type: str) -> else: logger.warning("Failed to persist branch_name metadata for thread %s", thread_id) + comment = payload.get("comment") or payload.get("review", {}) + is_review_request, _pr_url_override = parse_github_review_command(comment.get("body") or "") email = GITHUB_USER_EMAIL_MAP.get(github_login, "") - if not email: + if email: + github_token = await _get_or_resolve_thread_github_token(thread_id, email) + elif is_review_request: + github_token, expires_at = await get_github_app_installation_token_with_expiry() + if github_token: + try: + await persist_encrypted_github_token(thread_id, github_token, expires_at=expires_at) + except Exception: + logger.warning( + "Could not persist bot token for PR review request thread %s", thread_id + ) + else: logger.warning("No email mapping for GitHub user '%s', skipping", github_login) return - github_token = await _get_or_resolve_thread_github_token(thread_id, email) if not github_token: logger.warning("No GitHub token for thread %s, skipping", thread_id) return @@ -2634,14 +2627,6 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> "pull_request_review_comment", "pull_request_review", }: - is_review_command, pr_url_override = parse_github_review_command(comment_body) - if is_review_command: - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} - background_tasks.add_task( - process_github_pr_review_command, payload, event_type, pr_url_override - ) - return {"status": "accepted", "message": "Processing GitHub PR review command"} background_tasks.add_task(process_github_pr_comment, payload, event_type) return {"status": "accepted", "message": f"Processing {event_type} event"} diff --git a/tests/test_github_issue_webhook.py b/tests/test_github_issue_webhook.py index 59d16abc..b027c767 100644 --- a/tests/test_github_issue_webhook.py +++ b/tests/test_github_issue_webhook.py @@ -397,25 +397,31 @@ def test_github_webhook_ignores_review_requested_for_other_reviewer(monkeypatch) assert called is False -def test_slack_webhook_routes_review_command_to_reviewer(monkeypatch) -> None: +def test_slack_webhook_routes_review_command_to_agent(monkeypatch) -> None: captured: dict[str, object] = {} - async def fake_process_slack_pr_review_request( - pr_ref: GitHubPrRef, channel_id: str, thread_ts: str + async def fake_get_slack_repo_config( + channel_id: str, thread_ts: str, slack_user_id: str | None = None + ) -> dict[str, str]: + captured["repo_config_request"] = { + "channel_id": channel_id, + "thread_ts": thread_ts, + "slack_user_id": slack_user_id, + } + return {"owner": "langchain-ai", "name": "open-swe"} + + async def fake_process_slack_mention( + event_data: dict[str, object], repo_config: dict[str, str] ) -> None: - captured["pr_ref"] = pr_ref - captured["channel_id"] = channel_id - captured["thread_ts"] = thread_ts + captured["event_data"] = event_data + captured["repo_config"] = repo_config monkeypatch.setattr(webapp, "SLACK_SIGNING_SECRET", _TEST_SLACK_SECRET) monkeypatch.setattr(webapp, "SLACK_BOT_USER_ID", "UBOT") monkeypatch.setattr(webapp, "SLACK_BOT_USERNAME", "open-swe") monkeypatch.setattr(slack_utils.time, "time", lambda: 1700000000) - monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset()) - monkeypatch.setattr(webapp, "ALLOWED_REVIEWER_GITHUB_ORGS", frozenset()) - monkeypatch.setattr( - webapp, "process_slack_pr_review_request", fake_process_slack_pr_review_request - ) + monkeypatch.setattr(webapp, "get_slack_repo_config", fake_get_slack_repo_config) + monkeypatch.setattr(webapp, "process_slack_mention", fake_process_slack_mention) client = TestClient(webapp.app) response = _post_slack_webhook( @@ -433,32 +439,33 @@ def test_slack_webhook_routes_review_command_to_reviewer(monkeypatch) -> None: ) assert response.status_code == 200 - assert response.json()["message"] == "Slack PR review request queued" - pr_ref = captured["pr_ref"] - assert isinstance(pr_ref, GitHubPrRef) - assert pr_ref.owner == "langchain-ai" - assert pr_ref.repo == "open-swe" - assert pr_ref.number == 1244 - assert captured["channel_id"] == "C123" - assert captured["thread_ts"] == "1700000000.000100" + assert response.json()["message"] == "Slack mention queued" + assert captured["repo_config"] == {"owner": "langchain-ai", "name": "open-swe"} + event_data = captured["event_data"] + assert isinstance(event_data, dict) + assert event_data["text"] == "<@UBOT> review https://github.com/langchain-ai/open-swe/pull/1244" -def test_slack_webhook_malformed_review_command_does_not_start_agent(monkeypatch) -> None: +def test_slack_webhook_malformed_review_command_starts_agent(monkeypatch) -> None: captured: dict[str, object] = {} - async def fake_process_slack_mention(*args, **kwargs) -> None: - captured["agent_started"] = True + async def fake_get_slack_repo_config( + channel_id: str, thread_ts: str, slack_user_id: str | None = None + ) -> dict[str, str]: + return {"owner": "langchain-ai", "name": "open-swe"} - async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool: - captured["reply"] = text - return True + async def fake_process_slack_mention( + event_data: dict[str, object], repo_config: dict[str, str] + ) -> None: + captured["event_data"] = event_data + captured["repo_config"] = repo_config monkeypatch.setattr(webapp, "SLACK_SIGNING_SECRET", _TEST_SLACK_SECRET) monkeypatch.setattr(webapp, "SLACK_BOT_USER_ID", "UBOT") monkeypatch.setattr(webapp, "SLACK_BOT_USERNAME", "open-swe") monkeypatch.setattr(slack_utils.time, "time", lambda: 1700000000) + monkeypatch.setattr(webapp, "get_slack_repo_config", fake_get_slack_repo_config) monkeypatch.setattr(webapp, "process_slack_mention", fake_process_slack_mention) - monkeypatch.setattr(webapp, "post_slack_thread_reply", fake_post_slack_thread_reply) client = TestClient(webapp.app) response = _post_slack_webhook( @@ -476,9 +483,13 @@ def test_slack_webhook_malformed_review_command_does_not_start_agent(monkeypatch ) assert response.status_code == 200 - assert response.json()["reason"] == "Malformed Slack PR review command" - assert "agent_started" not in captured - assert "OWNER/REPO/pull/NUMBER" in captured["reply"] + assert response.json()["message"] == "Slack mention queued" + assert captured["repo_config"] == {"owner": "langchain-ai", "name": "open-swe"} + event_data = captured["event_data"] + assert isinstance(event_data, dict) + assert ( + event_data["text"] == "<@UBOT> review https://github.com/langchain-ai/open-swe/issues/1244" + ) def test_slack_webhook_non_pr_review_request_starts_agent(monkeypatch) -> None: @@ -886,24 +897,112 @@ def test_request_pr_review_tool_uses_shared_trigger(monkeypatch) -> None: source: str, github_login: str = "", github_user_id: int | None = None, + slack_channel_id: str = "", + slack_thread_ts: str = "", ) -> dict[str, object]: captured["pr_ref"] = pr_ref captured["source"] = source + captured["github_login"] = github_login + captured["github_user_id"] = github_user_id + captured["slack_channel_id"] = slack_channel_id + captured["slack_thread_ts"] = slack_thread_ts return {"success": True, "thread_id": "thread-id"} monkeypatch.setattr( request_pr_review_module, "trigger_pr_review_from_ref", fake_trigger_pr_review_from_ref ) + monkeypatch.setattr( + request_pr_review_module, + "get_config", + lambda: { + "configurable": { + "source": "github", + "github_login": "octocat", + "github_user_id": 123, + "slack_thread": {"channel_id": "C123", "thread_ts": "1700000000.000100"}, + } + }, + ) result = request_pr_review_tool("https://github.com/langchain-ai/open-swe/pull/1244") pr_ref = captured["pr_ref"] assert isinstance(pr_ref, GitHubPrRef) assert pr_ref.number == 1244 - assert captured["source"] == "slack" + assert captured["source"] == "github" + assert captured["github_login"] == "octocat" + assert captured["github_user_id"] == 123 + assert captured["slack_channel_id"] == "C123" + assert captured["slack_thread_ts"] == "1700000000.000100" assert result["success"] is True +def test_process_github_pr_comment_review_request_without_email_uses_app_token( + monkeypatch, +) -> None: + captured: dict[str, object] = {} + + async def fake_extract_pr_context(payload: dict[str, object], event_type: str): + return ( + {"owner": "langchain-ai", "name": "open-swe"}, + 1244, + "open-swe/00000000-0000-0000-0000-000000000001", + "external-user", + "https://github.com/langchain-ai/open-swe/pull/1244", + 9, + None, + ) + + async def fake_get_app_token_with_expiry() -> tuple[str, str]: + return "app-token", "2026-01-01T00:00:00Z" + + async def fake_persist_token( + thread_id: str, token: str, *, expires_at: str | None = None + ) -> str: + captured["persisted"] = {"thread_id": thread_id, "token": token, "expires_at": expires_at} + return "encrypted" + + async def fake_react(*args, **kwargs) -> bool: + captured["reaction_token"] = kwargs["token"] + return True + + async def fake_fetch_comments(repo_config: dict[str, str], pr_number: int, *, token: str): + captured["fetch_token"] = token + return [{"body": "@open-swe review", "author": "external-user", "created_at": "now"}] + + async def fake_trigger_or_queue_run(*args, **kwargs) -> None: + captured["triggered"] = {"args": args, "kwargs": kwargs} + + monkeypatch.setattr(webapp, "extract_pr_context", fake_extract_pr_context) + monkeypatch.setattr(webapp, "GITHUB_USER_EMAIL_MAP", {}) + monkeypatch.setattr( + webapp, "get_github_app_installation_token_with_expiry", fake_get_app_token_with_expiry + ) + monkeypatch.setattr(webapp, "persist_encrypted_github_token", fake_persist_token) + monkeypatch.setattr(webapp, "react_to_github_comment", fake_react) + monkeypatch.setattr(webapp, "fetch_pr_comments_since_last_tag", fake_fetch_comments) + monkeypatch.setattr(webapp, "_trigger_or_queue_run", fake_trigger_or_queue_run) + + asyncio.run( + webapp.process_github_pr_comment( + { + "comment": {"id": 9, "body": "@open-swe review"}, + "sender": {"login": "external-user", "id": 123}, + }, + "issue_comment", + ) + ) + + assert captured["reaction_token"] == "app-token" + assert captured["fetch_token"] == "app-token" + assert captured["persisted"] == { + "thread_id": "00000000-0000-0000-0000-000000000001", + "token": "app-token", + "expires_at": "2026-01-01T00:00:00Z", + } + assert captured["triggered"] + + def test_process_github_issue_uses_resolved_user_token_for_reaction(monkeypatch) -> None: captured: dict[str, object] = {} @@ -1105,25 +1204,21 @@ def test_parse_github_review_command_does_not_swallow_trailing_text() -> None: ) -def test_github_webhook_routes_pr_comment_review_to_reviewer(monkeypatch) -> None: +def test_github_webhook_routes_pr_comment_review_to_agent(monkeypatch) -> None: captured: dict[str, object] = {} async def fake_process_pr_comment(payload: dict[str, object], event_type: str) -> None: - raise AssertionError("process_github_pr_comment should not be called for review command") + captured["payload"] = payload + captured["event_type"] = event_type async def fake_process_review_command( payload: dict[str, object], event_type: str, pr_url_override: str | None ) -> None: - captured["payload"] = payload - captured["event_type"] = event_type - captured["pr_url_override"] = pr_url_override + raise AssertionError("review commands should route through the main agent") monkeypatch.setattr(webapp, "process_github_pr_comment", fake_process_pr_comment) monkeypatch.setattr(webapp, "process_github_pr_review_command", fake_process_review_command) monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) - monkeypatch.setattr( - webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) - ) monkeypatch.setattr(webapp, "ALLOWED_GITHUB_ORGS", frozenset({"langchain-ai"})) client = TestClient(webapp.app) @@ -1144,26 +1239,31 @@ def test_github_webhook_routes_pr_comment_review_to_reviewer(monkeypatch) -> Non ) assert response.status_code == 200 - assert response.json() == { - "status": "accepted", - "message": "Processing GitHub PR review command", - } + assert response.json() == {"status": "accepted", "message": "Processing issue_comment event"} assert captured["event_type"] == "issue_comment" - assert captured["pr_url_override"] is None -def test_github_webhook_blocks_pr_review_command_outside_reviewer_allowlist(monkeypatch) -> None: +def test_github_webhook_routes_pr_review_request_outside_reviewer_allowlist_to_agent( + monkeypatch, +) -> None: + captured: dict[str, object] = {} + + async def fake_process_pr_comment(payload: dict[str, object], event_type: str) -> None: + captured["payload"] = payload + captured["event_type"] = event_type + async def fake_process_review_command( payload: dict[str, object], event_type: str, pr_url_override: str | None ) -> None: - raise AssertionError("process_github_pr_review_command should not run") + raise AssertionError("review allowlist is enforced by request_pr_review") + monkeypatch.setattr(webapp, "process_github_pr_comment", fake_process_pr_comment) monkeypatch.setattr(webapp, "process_github_pr_review_command", fake_process_review_command) monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) + monkeypatch.setattr(webapp, "ALLOWED_GITHUB_ORGS", frozenset({"langchain-ai"})) monkeypatch.setattr( webapp, "ALLOWED_REVIEWER_GITHUB_REPOS", frozenset({"langchain-ai/open-swe"}) ) - monkeypatch.setattr(webapp, "ALLOWED_GITHUB_ORGS", frozenset({"langchain-ai"})) client = TestClient(webapp.app) response = _post_github_webhook( @@ -1183,10 +1283,8 @@ def test_github_webhook_blocks_pr_review_command_outside_reviewer_allowlist(monk ) assert response.status_code == 200 - assert response.json() == { - "status": "ignored", - "reason": "Repository not enabled for review", - } + assert response.json() == {"status": "accepted", "message": "Processing issue_comment event"} + assert captured["event_type"] == "issue_comment" def test_process_github_pr_review_command_uses_payload_pr(monkeypatch) -> None: diff --git a/tests/test_public_repo_org_gate.py b/tests/test_public_repo_org_gate.py index 87a93d1c..0df971d1 100644 --- a/tests/test_public_repo_org_gate.py +++ b/tests/test_public_repo_org_gate.py @@ -104,12 +104,10 @@ def test_gate_allows_org_member_on_public_pr_comment(monkeypatch) -> None: called: dict[str, object] = {} - async def fake_process_github_pr_review_command(payload, event_type, pr_url_override) -> None: + async def fake_process_github_pr_comment(payload, event_type) -> None: called["event"] = event_type - monkeypatch.setattr( - webapp, "process_github_pr_review_command", fake_process_github_pr_review_command - ) + monkeypatch.setattr(webapp, "process_github_pr_comment", fake_process_github_pr_comment) client = TestClient(webapp.app) response = _post_github_webhook(