diff --git a/agent/webapp.py b/agent/webapp.py index 407301df..7a21085e 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -646,12 +646,8 @@ def _is_repo_allowed(repo_config: dict[str, str]) -> bool: return False -async def _is_repo_enabled_for_review(repo_config: dict[str, str]) -> bool: - """Check the dashboard opt-in list for reviewer-agent entrypoints. - - The opt-in list is empty by default, so repos are off until an admin - enables them in the dashboard's Open SWE Review tab. - """ +async def _is_repo_auto_review_enabled(repo_config: dict[str, str]) -> bool: + """Return whether automatic reviews are enabled for a repository.""" return await is_review_repo_enabled(repo_config.get("owner", ""), repo_config.get("name", "")) @@ -2421,14 +2417,12 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> if action in _GH_PR_AGENT_STATE_ACTIONS: background_tasks.add_task(update_agent_thread_pr_state, payload) if action in _GH_PR_WATCH_TOGGLE_ACTIONS: - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} logger.info("Accepted GitHub PR %s webhook, scheduling reviewer watch update", action) background_tasks.add_task(process_github_pr_close, payload) return {"status": "accepted", "message": f"Processing PR {action} for reviewer watch"} if action in _GH_PR_FIRST_REVIEW_ACTIONS: - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} + if not await _is_repo_auto_review_enabled(webhook_repo_config): + return {"status": "ignored", "reason": "Automatic review disabled for repository"} gate_rejection = await _enforce_public_repo_org_gate(payload, "pull_request") if gate_rejection is not None: return gate_rejection @@ -2442,8 +2436,8 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> } if event_type == "push": - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} + if not await _is_repo_auto_review_enabled(webhook_repo_config): + return {"status": "ignored", "reason": "Automatic review disabled for repository"} logger.info("Accepted GitHub push webhook, scheduling reviewer watch evaluation") background_tasks.add_task(process_github_push_event, payload) return {"status": "accepted", "message": "Processing GitHub push for reviewer watch"} @@ -2451,8 +2445,8 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> if event_type in _GH_CI_EVENTS: if not is_failing_ci_payload(payload, event_type): return {"status": "ignored", "reason": "CI event is not a completed failure"} - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} + if not await _is_repo_auto_review_enabled(webhook_repo_config): + return {"status": "ignored", "reason": "Automatic review disabled for repository"} logger.info("Accepted GitHub %s webhook, scheduling CI auto-fix evaluation", event_type) background_tasks.add_task(process_github_ci_event, payload, event_type) return {"status": "accepted", "message": f"Processing GitHub {event_type} for auto-fix"} @@ -2507,8 +2501,8 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> } autofix_command = _parse_autofix_command(comment_body) if autofix_command is not None and is_pr_related_comment: - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} + if not await _is_repo_auto_review_enabled(webhook_repo_config): + return {"status": "ignored", "reason": "Automatic review disabled for repository"} gate_rejection = await _enforce_public_repo_org_gate(payload, event_type) if gate_rejection is not None: return gate_rejection @@ -2521,8 +2515,6 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> event_type == "pull_request_review_comment" and _review_comment_reply_parent_id(payload) is not None ): - if not await _is_repo_enabled_for_review(webhook_repo_config): - return {"status": "ignored", "reason": "Repository not enabled for review"} gate_rejection = await _enforce_public_repo_org_gate(payload, event_type) if gate_rejection is not None: return gate_rejection @@ -2530,9 +2522,9 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> return {"status": "accepted", "message": "Processing review finding reply"} if not any(tag in comment_body.lower() for tag in OPEN_SWE_TAGS): - if _is_actionable_review_payload(payload, event_type) and await _is_repo_enabled_for_review( - webhook_repo_config - ): + if _is_actionable_review_payload( + payload, event_type + ) and await _is_repo_auto_review_enabled(webhook_repo_config): gate_rejection = await _enforce_public_repo_org_gate(payload, event_type) if gate_rejection is not None: return gate_rejection diff --git a/agent/webhooks/github.py b/agent/webhooks/github.py index 960cf102..5da49f03 100644 --- a/agent/webhooks/github.py +++ b/agent/webhooks/github.py @@ -107,8 +107,6 @@ async def trigger_pr_review_from_ref( slack_thread_ts: str = "", ) -> dict[str, Any]: repo_config = {"owner": pr_ref.owner, "name": pr_ref.repo} - if not await webapp._is_repo_enabled_for_review(repo_config): - return {"success": False, "error": "Repository not enabled for review"} # Full token to read PR metadata (privacy/id aren't in the trigger ref); # re-scoped below once we know whether the repo is public. @@ -368,8 +366,6 @@ async def process_github_pr_close(payload: dict[str, Any]) -> None: pr_number = pull_request.get("number") if not pr_number or not isinstance(pr_number, int): return - if not await webapp._is_repo_enabled_for_review(repo_config): - return thread_id = webapp.generate_reviewer_thread_id( repo_config.get("owner", ""), repo_config.get("name", ""), pr_number @@ -432,9 +428,9 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: "Push to %s ignored: repository owner/name missing from payload", head_ref ) return - if not await webapp._is_repo_enabled_for_review(repo_config): + if not await webapp._is_repo_auto_review_enabled(repo_config): webapp.logger.info( - "Push to %s/%s head=%s ignored: repo not enabled for review", + "Push to %s/%s head=%s ignored: automatic review disabled", repo_config["owner"], repo_config["name"], head_ref, diff --git a/tests/conftest.py b/tests/conftest.py index 94241b6e..09ffe6b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,13 +8,13 @@ from agent import webapp @pytest.fixture(autouse=True) -def _default_enable_review_repos(monkeypatch: pytest.MonkeyPatch) -> None: - """Treat every repo as enabled for review by default. +def _default_enable_auto_review(monkeypatch: pytest.MonkeyPatch) -> None: + """Treat automatic reviews as enabled for every repo by default. The dashboard's opt-in list (loaded by :func:`agent.dashboard.enabled_repos.is_review_repo_enabled`) is empty in the test environment because there is no live LangGraph Store. - Tests targeting the opt-in gate itself should override this fixture or set + Tests targeting the automatic-review gate should override this fixture or set ``monkeypatch.setattr(webapp, "is_review_repo_enabled", ...)`` to a stricter stub. """ diff --git a/tests/test_github_issue_webhook.py b/tests/test_github_issue_webhook.py index 8ada3abc..17e52820 100644 --- a/tests/test_github_issue_webhook.py +++ b/tests/test_github_issue_webhook.py @@ -105,7 +105,7 @@ def test_build_github_issue_followup_prompt_only_includes_comment() -> None: assert "## Title" not in prompt -def test_reviewer_enablement_uses_dashboard_opt_in(monkeypatch) -> None: +def test_auto_review_enablement_uses_dashboard_opt_in(monkeypatch) -> None: seen: dict[str, str] = {} async def fake_is_review_repo_enabled(owner: str, name: str) -> bool: @@ -117,19 +117,52 @@ def test_reviewer_enablement_uses_dashboard_opt_in(monkeypatch) -> None: assert ( asyncio.run( - webapp._is_repo_enabled_for_review({"owner": "langchain-ai", "name": "open-swe-app"}) + webapp._is_repo_auto_review_enabled({"owner": "langchain-ai", "name": "open-swe-app"}) ) is True ) assert seen == {"owner": "langchain-ai", "name": "open-swe-app"} assert ( asyncio.run( - webapp._is_repo_enabled_for_review({"owner": "langchain-ai", "name": "open-swe"}) + webapp._is_repo_auto_review_enabled({"owner": "langchain-ai", "name": "open-swe"}) ) is False ) +def test_github_webhook_skips_automatic_review_when_disabled(monkeypatch) -> None: + called = False + + async def fake_auto_review_enabled(_repo_config: dict[str, str]) -> bool: + return False + + async def fake_process_github_pr_ready(_payload: dict[str, object]) -> None: + nonlocal called + called = True + + monkeypatch.setattr(webapp, "_is_repo_auto_review_enabled", fake_auto_review_enabled) + monkeypatch.setattr(webapp, "process_github_pr_ready", fake_process_github_pr_ready) + monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) + + client = TestClient(webapp.app) + response = _post_github_webhook( + client, + "pull_request", + { + "action": "opened", + "repository": {"owner": {"login": "langchain-ai"}, "name": "open-swe"}, + "pull_request": {"number": 1244}, + }, + ) + + assert response.status_code == 200 + assert response.json() == { + "status": "ignored", + "reason": "Automatic review disabled for repository", + } + assert called is False + + def test_github_webhook_accepts_issue_events(monkeypatch) -> None: called: dict[str, object] = {} @@ -259,17 +292,20 @@ def test_github_webhook_ignores_unmentioned_comment_without_info_log(monkeypatch def test_github_webhook_routes_review_comment_reply_without_tag(monkeypatch) -> None: called: dict[str, object] = {} + auto_review_checked = False async def fake_process_github_review_finding_reply(payload: dict[str, object]) -> None: called["payload"] = payload - async def fake_repo_enabled(_repo_config: dict[str, str]) -> bool: - return True + async def fake_auto_review_enabled(_repo_config: dict[str, str]) -> bool: + nonlocal auto_review_checked + auto_review_checked = True + return False monkeypatch.setattr( webapp, "process_github_review_finding_reply", fake_process_github_review_finding_reply ) - monkeypatch.setattr(webapp, "_is_repo_enabled_for_review", fake_repo_enabled) + monkeypatch.setattr(webapp, "_is_repo_auto_review_enabled", fake_auto_review_enabled) monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET) client = TestClient(webapp.app) @@ -295,6 +331,7 @@ def test_github_webhook_routes_review_comment_reply_without_tag(monkeypatch) -> assert response.status_code == 200 assert response.json()["status"] == "accepted" + assert auto_review_checked is False payload = called["payload"] assert isinstance(payload, dict) assert payload["comment"]["in_reply_to_id"] == 111 @@ -917,6 +954,12 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None: def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: captured: dict[str, object] = {} + auto_review_checked = False + + async def fake_auto_review_enabled(_repo_config: dict[str, str]) -> bool: + nonlocal auto_review_checked + auto_review_checked = True + return False async def fake_get_github_app_installation_token() -> str | None: return "app-token" @@ -959,6 +1002,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: captured["set_metadata_thread_id"] = thread_id captured["set_metadata_kwargs"] = kwargs + monkeypatch.setattr(webapp, "_is_repo_auto_review_enabled", fake_auto_review_enabled) monkeypatch.setattr( webapp, "get_github_app_installation_token", fake_get_github_app_installation_token ) @@ -996,6 +1040,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: prompt = kwargs["input"]["messages"][0]["content"] config = kwargs["config"]["configurable"] assert result["success"] is True + assert auto_review_checked is False assert captured["graph"] == "reviewer" assert captured["thread_create_kwargs"] == { "thread_id": captured["thread_id"], @@ -1019,39 +1064,6 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: assert captured["status_comment_kwargs"]["pr_number"] == 1244 -def test_trigger_pr_review_from_ref_respects_dashboard_opt_in(monkeypatch) -> None: - called = False - - async def fake_get_github_app_installation_token() -> str | None: - nonlocal called - called = True - return "app-token" - - monkeypatch.setattr( - webapp, "get_github_app_installation_token", fake_get_github_app_installation_token - ) - - async def fake_is_review_repo_enabled(_owner: str, _name: str) -> bool: - return False - - monkeypatch.setattr(webapp, "is_review_repo_enabled", fake_is_review_repo_enabled) - - result = asyncio.run( - webapp.trigger_pr_review_from_ref( - GitHubPrRef( - owner="langchain-ai", - repo="blocked", - number=1, - url="https://github.com/langchain-ai/blocked/pull/1", - ), - source="slack", - ) - ) - - assert result == {"success": False, "error": "Repository not enabled for review"} - assert called is False - - async def test_request_pr_review_tool_uses_shared_trigger(monkeypatch) -> None: captured: dict[str, object] = {} diff --git a/tests/test_reviewer_watch.py b/tests/test_reviewer_watch.py index ad3b74bc..8c68a776 100644 --- a/tests/test_reviewer_watch.py +++ b/tests/test_reviewer_watch.py @@ -46,7 +46,7 @@ async def test_push_event_skips_branch_deletion() -> None: ref="refs/heads/feat-x", after="0000000000000000000000000000000000000000" ) with patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ): await webapp.process_github_push_event(payload) # If we got here without crashing and with no other patches needed, the @@ -68,7 +68,7 @@ async def test_push_event_skips_when_thread_not_watching() -> None: with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch( "agent.webapp.get_github_app_installation_token", @@ -107,7 +107,7 @@ async def test_push_event_skips_when_pr_diff_unchanged_since_last_review() -> No with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch( "agent.webapp.get_github_app_installation_token_with_expiry", @@ -175,7 +175,7 @@ async def test_push_event_triggers_re_review_run_when_watching() -> None: with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch( "agent.webapp.get_github_app_installation_token", @@ -267,7 +267,7 @@ async def test_push_event_idempotent_when_head_unchanged() -> None: with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch( "agent.webapp.get_github_app_installation_token", @@ -352,7 +352,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None: with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token), patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr), @@ -396,7 +396,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=True ), patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token), patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr), @@ -432,8 +432,8 @@ async def test_pr_close_disables_watch() -> None: with ( patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True - ), + "agent.webapp._is_repo_auto_review_enabled", new_callable=AsyncMock, return_value=False + ) as auto_review_enabled, patch( "agent.webapp._get_thread_metadata_safe", new_callable=AsyncMock, @@ -442,6 +442,7 @@ async def test_pr_close_disables_watch() -> None: patch("agent.webapp.set_reviewer_thread_metadata", side_effect=fake_set), ): await webapp.process_github_pr_close(_pr_close_payload(action="closed")) + auto_review_enabled.assert_not_awaited() assert captured and captured[0][1]["watch"] is False @@ -453,9 +454,6 @@ async def test_pr_reopened_re_enables_watch() -> None: captured.append((thread_id, kwargs)) with ( - patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True - ), patch( "agent.webapp._get_thread_metadata_safe", new_callable=AsyncMock, @@ -471,9 +469,6 @@ async def test_pr_reopened_re_enables_watch() -> None: async def test_pr_close_skips_non_reviewer_threads() -> None: fake_set = AsyncMock() with ( - patch( - "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True - ), patch( "agent.webapp._get_thread_metadata_safe", new_callable=AsyncMock, diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index fe0652db..a2115a95 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -733,12 +733,12 @@ export const api = { request("/my-credentials/notion", { method: "DELETE", }), - listEnabledReviewRepos: () => + listAutoReviewRepos: () => request<{ repos: Array }>("/enabled-review-repos"), - setEnabledReviewRepo: (full_name: string, enabled: boolean) => + setAutoReviewRepo: (full_name: string, runAutomatically: boolean) => request<{ repos: Array }>("/enabled-review-repos", { method: "PUT", - body: JSON.stringify({ full_name, enabled }), + body: JSON.stringify({ full_name, enabled: runAutomatically }), }), usageLeaderboard: (period: UsageLeaderboardPeriod = "30d", limit = 10) => request( diff --git a/ui/src/routes/review.tsx b/ui/src/routes/review.tsx index 91ee968e..1f1a75fc 100644 --- a/ui/src/routes/review.tsx +++ b/ui/src/routes/review.tsx @@ -91,7 +91,7 @@ function ReviewPage() { @@ -202,14 +202,14 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) { }, }); - const enabled = useQuery({ - queryKey: ["enabledReviewRepos"], - queryFn: api.listEnabledReviewRepos, + const autoReview = useQuery({ + queryKey: ["autoReviewRepos"], + queryFn: api.listAutoReviewRepos, }); - const enabledSet = useMemo( - () => new Set(enabled.data?.repos ?? []), - [enabled.data?.repos], + const autoReviewSet = useMemo( + () => new Set(autoReview.data?.repos ?? []), + [autoReview.data?.repos], ); const grouped = useMemo(() => { @@ -224,12 +224,12 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) { return Array.from(byOwner.entries()).sort(([a], [b]) => a.localeCompare(b)); }, [repos.data?.repositories]); - const loading = repos.isLoading || enabled.isLoading; + const loading = repos.isLoading || autoReview.isLoading; return (
{loading && ( @@ -244,7 +244,7 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) {

)} {grouped.map(([owner, list]) => { - const enabledCount = list.filter((r) => enabledSet.has(r.full_name)).length; + const autoReviewCount = list.filter((r) => autoReviewSet.has(r.full_name)).length; return (
- {enabledCount}/{list.length} Repositories Enabled + {autoReviewCount}/{list.length} Run Automatically
diff --git a/ui/src/routes/review_.repositories.$owner.tsx b/ui/src/routes/review_.repositories.$owner.tsx index 64c8e797..42d120a3 100644 --- a/ui/src/routes/review_.repositories.$owner.tsx +++ b/ui/src/routes/review_.repositories.$owner.tsx @@ -36,17 +36,17 @@ function RepositoriesOwnerPage() { enabled: !!session.data, }); - const enabled = useQuery({ - queryKey: ["enabledReviewRepos"], - queryFn: api.listEnabledReviewRepos, + const autoReview = useQuery({ + queryKey: ["autoReviewRepos"], + queryFn: api.listAutoReviewRepos, enabled: !!session.data, }); - const toggle = useMutation({ + const toggleAutoReview = useMutation({ mutationFn: ({ full_name, on }: { full_name: string; on: boolean }) => - api.setEnabledReviewRepo(full_name, on), + api.setAutoReviewRepo(full_name, on), onSuccess: (data) => { - qc.setQueryData(["enabledReviewRepos"], data); + qc.setQueryData(["autoReviewRepos"], data); }, }); @@ -58,9 +58,9 @@ function RepositoriesOwnerPage() { [repos.data?.repositories, owner], ); - const enabledSet = useMemo( - () => new Set(enabled.data?.repos ?? []), - [enabled.data?.repos], + const autoReviewSet = useMemo( + () => new Set(autoReview.data?.repos ?? []), + [autoReview.data?.repos], ); const [page, setPage] = useState(0); @@ -82,8 +82,8 @@ function RepositoriesOwnerPage() { if (!session.data) return ; const canEdit = session.data.is_admin; - const enabledCount = ownerRepos.filter((r) => enabledSet.has(r.full_name)).length; - const loading = repos.isLoading || enabled.isLoading; + const autoReviewCount = ownerRepos.filter((r) => autoReviewSet.has(r.full_name)).length; + const loading = repos.isLoading || autoReview.isLoading; return ( @@ -102,7 +102,7 @@ function RepositoriesOwnerPage() { Repositories - {enabledCount}/{ownerRepos.length} enabled + {autoReviewCount}/{ownerRepos.length} run automatically
@@ -118,7 +118,7 @@ function RepositoriesOwnerPage() { )}
    {pageRepos.map((r) => { - const isEnabled = enabledSet.has(r.full_name); + const runsAutomatically = autoReviewSet.has(r.full_name); return (
  • private )}
- - - toggle.mutate({ full_name: r.full_name, on: v }) +
+ Run automatically + - + className={!canEdit ? "cursor-not-allowed" : undefined} + > + + toggleAutoReview.mutate({ full_name: r.full_name, on: v }) + } + /> + +
); })}