mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-04 14:52:12 +00:00
* fix: separate review access from automatic reviews (#1720) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> (cherry picked from commit 92d631704e9c38f6aeedf70baede85a7c638567e) * fix: rename fork-only auto-fix gates to _is_repo_auto_review_enabled The cherry-pick of upstream 92d63170 renamed _is_repo_enabled_for_review to _is_repo_auto_review_enabled, but the fork's CI auto-fix, auto-fix toggle command, and auto-fix review-feedback gates (not present upstream) still referenced the old name and would raise NameError at request time. Rename them in place — auto-fix surfaces stay gated on the dashboard auto-review opt-in list, preserving current fork behavior. --------- Co-authored-by: Johannes du Plessis <johannes@langchain.dev> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
032d3889e4
commit
c80bd0f92f
8 changed files with 127 additions and 128 deletions
|
|
@ -646,12 +646,8 @@ def _is_repo_allowed(repo_config: dict[str, str]) -> bool:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
async def _is_repo_enabled_for_review(repo_config: dict[str, str]) -> bool:
|
async def _is_repo_auto_review_enabled(repo_config: dict[str, str]) -> bool:
|
||||||
"""Check the dashboard opt-in list for reviewer-agent entrypoints.
|
"""Return whether automatic reviews are enabled for a repository."""
|
||||||
|
|
||||||
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.
|
|
||||||
"""
|
|
||||||
return await is_review_repo_enabled(repo_config.get("owner", ""), repo_config.get("name", ""))
|
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:
|
if action in _GH_PR_AGENT_STATE_ACTIONS:
|
||||||
background_tasks.add_task(update_agent_thread_pr_state, payload)
|
background_tasks.add_task(update_agent_thread_pr_state, payload)
|
||||||
if action in _GH_PR_WATCH_TOGGLE_ACTIONS:
|
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)
|
logger.info("Accepted GitHub PR %s webhook, scheduling reviewer watch update", action)
|
||||||
background_tasks.add_task(process_github_pr_close, payload)
|
background_tasks.add_task(process_github_pr_close, payload)
|
||||||
return {"status": "accepted", "message": f"Processing PR {action} for reviewer watch"}
|
return {"status": "accepted", "message": f"Processing PR {action} for reviewer watch"}
|
||||||
if action in _GH_PR_FIRST_REVIEW_ACTIONS:
|
if action in _GH_PR_FIRST_REVIEW_ACTIONS:
|
||||||
if not await _is_repo_enabled_for_review(webhook_repo_config):
|
if not await _is_repo_auto_review_enabled(webhook_repo_config):
|
||||||
return {"status": "ignored", "reason": "Repository not enabled for review"}
|
return {"status": "ignored", "reason": "Automatic review disabled for repository"}
|
||||||
gate_rejection = await _enforce_public_repo_org_gate(payload, "pull_request")
|
gate_rejection = await _enforce_public_repo_org_gate(payload, "pull_request")
|
||||||
if gate_rejection is not None:
|
if gate_rejection is not None:
|
||||||
return gate_rejection
|
return gate_rejection
|
||||||
|
|
@ -2442,8 +2436,8 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
||||||
}
|
}
|
||||||
|
|
||||||
if event_type == "push":
|
if event_type == "push":
|
||||||
if not await _is_repo_enabled_for_review(webhook_repo_config):
|
if not await _is_repo_auto_review_enabled(webhook_repo_config):
|
||||||
return {"status": "ignored", "reason": "Repository not enabled for review"}
|
return {"status": "ignored", "reason": "Automatic review disabled for repository"}
|
||||||
logger.info("Accepted GitHub push webhook, scheduling reviewer watch evaluation")
|
logger.info("Accepted GitHub push webhook, scheduling reviewer watch evaluation")
|
||||||
background_tasks.add_task(process_github_push_event, payload)
|
background_tasks.add_task(process_github_push_event, payload)
|
||||||
return {"status": "accepted", "message": "Processing GitHub push for reviewer watch"}
|
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 event_type in _GH_CI_EVENTS:
|
||||||
if not is_failing_ci_payload(payload, event_type):
|
if not is_failing_ci_payload(payload, event_type):
|
||||||
return {"status": "ignored", "reason": "CI event is not a completed failure"}
|
return {"status": "ignored", "reason": "CI event is not a completed failure"}
|
||||||
if not await _is_repo_enabled_for_review(webhook_repo_config):
|
if not await _is_repo_auto_review_enabled(webhook_repo_config):
|
||||||
return {"status": "ignored", "reason": "Repository not enabled for review"}
|
return {"status": "ignored", "reason": "Automatic review disabled for repository"}
|
||||||
logger.info("Accepted GitHub %s webhook, scheduling CI auto-fix evaluation", event_type)
|
logger.info("Accepted GitHub %s webhook, scheduling CI auto-fix evaluation", event_type)
|
||||||
background_tasks.add_task(process_github_ci_event, payload, 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"}
|
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)
|
autofix_command = _parse_autofix_command(comment_body)
|
||||||
if autofix_command is not None and is_pr_related_comment:
|
if autofix_command is not None and is_pr_related_comment:
|
||||||
if not await _is_repo_enabled_for_review(webhook_repo_config):
|
if not await _is_repo_auto_review_enabled(webhook_repo_config):
|
||||||
return {"status": "ignored", "reason": "Repository not enabled for review"}
|
return {"status": "ignored", "reason": "Automatic review disabled for repository"}
|
||||||
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
||||||
if gate_rejection is not None:
|
if gate_rejection is not None:
|
||||||
return gate_rejection
|
return gate_rejection
|
||||||
|
|
@ -2521,8 +2515,6 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
||||||
event_type == "pull_request_review_comment"
|
event_type == "pull_request_review_comment"
|
||||||
and _review_comment_reply_parent_id(payload) is not None
|
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)
|
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
||||||
if gate_rejection is not None:
|
if gate_rejection is not None:
|
||||||
return gate_rejection
|
return gate_rejection
|
||||||
|
|
@ -2530,9 +2522,9 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
||||||
return {"status": "accepted", "message": "Processing review finding reply"}
|
return {"status": "accepted", "message": "Processing review finding reply"}
|
||||||
|
|
||||||
if not any(tag in comment_body.lower() for tag in OPEN_SWE_TAGS):
|
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(
|
if _is_actionable_review_payload(
|
||||||
webhook_repo_config
|
payload, event_type
|
||||||
):
|
) and await _is_repo_auto_review_enabled(webhook_repo_config):
|
||||||
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
||||||
if gate_rejection is not None:
|
if gate_rejection is not None:
|
||||||
return gate_rejection
|
return gate_rejection
|
||||||
|
|
|
||||||
|
|
@ -107,8 +107,6 @@ async def trigger_pr_review_from_ref(
|
||||||
slack_thread_ts: str = "",
|
slack_thread_ts: str = "",
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
repo_config = {"owner": pr_ref.owner, "name": pr_ref.repo}
|
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);
|
# 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.
|
# 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")
|
pr_number = pull_request.get("number")
|
||||||
if not pr_number or not isinstance(pr_number, int):
|
if not pr_number or not isinstance(pr_number, int):
|
||||||
return
|
return
|
||||||
if not await webapp._is_repo_enabled_for_review(repo_config):
|
|
||||||
return
|
|
||||||
|
|
||||||
thread_id = webapp.generate_reviewer_thread_id(
|
thread_id = webapp.generate_reviewer_thread_id(
|
||||||
repo_config.get("owner", ""), repo_config.get("name", ""), pr_number
|
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
|
"Push to %s ignored: repository owner/name missing from payload", head_ref
|
||||||
)
|
)
|
||||||
return
|
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(
|
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["owner"],
|
||||||
repo_config["name"],
|
repo_config["name"],
|
||||||
head_ref,
|
head_ref,
|
||||||
|
|
|
||||||
|
|
@ -8,13 +8,13 @@ from agent import webapp
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _default_enable_review_repos(monkeypatch: pytest.MonkeyPatch) -> None:
|
def _default_enable_auto_review(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
"""Treat every repo as enabled for review by default.
|
"""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`)
|
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.
|
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.
|
``monkeypatch.setattr(webapp, "is_review_repo_enabled", ...)`` to a stricter stub.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -105,7 +105,7 @@ def test_build_github_issue_followup_prompt_only_includes_comment() -> None:
|
||||||
assert "## Title" not in prompt
|
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] = {}
|
seen: dict[str, str] = {}
|
||||||
|
|
||||||
async def fake_is_review_repo_enabled(owner: str, name: str) -> bool:
|
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 (
|
assert (
|
||||||
asyncio.run(
|
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
|
is True
|
||||||
)
|
)
|
||||||
assert seen == {"owner": "langchain-ai", "name": "open-swe-app"}
|
assert seen == {"owner": "langchain-ai", "name": "open-swe-app"}
|
||||||
assert (
|
assert (
|
||||||
asyncio.run(
|
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
|
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:
|
def test_github_webhook_accepts_issue_events(monkeypatch) -> None:
|
||||||
called: dict[str, object] = {}
|
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:
|
def test_github_webhook_routes_review_comment_reply_without_tag(monkeypatch) -> None:
|
||||||
called: dict[str, object] = {}
|
called: dict[str, object] = {}
|
||||||
|
auto_review_checked = False
|
||||||
|
|
||||||
async def fake_process_github_review_finding_reply(payload: dict[str, object]) -> None:
|
async def fake_process_github_review_finding_reply(payload: dict[str, object]) -> None:
|
||||||
called["payload"] = payload
|
called["payload"] = payload
|
||||||
|
|
||||||
async def fake_repo_enabled(_repo_config: dict[str, str]) -> bool:
|
async def fake_auto_review_enabled(_repo_config: dict[str, str]) -> bool:
|
||||||
return True
|
nonlocal auto_review_checked
|
||||||
|
auto_review_checked = True
|
||||||
|
return False
|
||||||
|
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
webapp, "process_github_review_finding_reply", fake_process_github_review_finding_reply
|
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)
|
monkeypatch.setattr(webapp, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||||
|
|
||||||
client = TestClient(webapp.app)
|
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.status_code == 200
|
||||||
assert response.json()["status"] == "accepted"
|
assert response.json()["status"] == "accepted"
|
||||||
|
assert auto_review_checked is False
|
||||||
payload = called["payload"]
|
payload = called["payload"]
|
||||||
assert isinstance(payload, dict)
|
assert isinstance(payload, dict)
|
||||||
assert payload["comment"]["in_reply_to_id"] == 111
|
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:
|
def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None:
|
||||||
captured: dict[str, object] = {}
|
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:
|
async def fake_get_github_app_installation_token() -> str | None:
|
||||||
return "app-token"
|
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_thread_id"] = thread_id
|
||||||
captured["set_metadata_kwargs"] = kwargs
|
captured["set_metadata_kwargs"] = kwargs
|
||||||
|
|
||||||
|
monkeypatch.setattr(webapp, "_is_repo_auto_review_enabled", fake_auto_review_enabled)
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
webapp, "get_github_app_installation_token", fake_get_github_app_installation_token
|
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"]
|
prompt = kwargs["input"]["messages"][0]["content"]
|
||||||
config = kwargs["config"]["configurable"]
|
config = kwargs["config"]["configurable"]
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
|
assert auto_review_checked is False
|
||||||
assert captured["graph"] == "reviewer"
|
assert captured["graph"] == "reviewer"
|
||||||
assert captured["thread_create_kwargs"] == {
|
assert captured["thread_create_kwargs"] == {
|
||||||
"thread_id": captured["thread_id"],
|
"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
|
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:
|
async def test_request_pr_review_tool_uses_shared_trigger(monkeypatch) -> None:
|
||||||
captured: dict[str, object] = {}
|
captured: dict[str, object] = {}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -46,7 +46,7 @@ async def test_push_event_skips_branch_deletion() -> None:
|
||||||
ref="refs/heads/feat-x", after="0000000000000000000000000000000000000000"
|
ref="refs/heads/feat-x", after="0000000000000000000000000000000000000000"
|
||||||
)
|
)
|
||||||
with patch(
|
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)
|
await webapp.process_github_push_event(payload)
|
||||||
# If we got here without crashing and with no other patches needed, the
|
# 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 (
|
with (
|
||||||
patch(
|
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(
|
patch(
|
||||||
"agent.webapp.get_github_app_installation_token",
|
"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 (
|
with (
|
||||||
patch(
|
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(
|
patch(
|
||||||
"agent.webapp.get_github_app_installation_token_with_expiry",
|
"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 (
|
with (
|
||||||
patch(
|
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(
|
patch(
|
||||||
"agent.webapp.get_github_app_installation_token",
|
"agent.webapp.get_github_app_installation_token",
|
||||||
|
|
@ -267,7 +267,7 @@ async def test_push_event_idempotent_when_head_unchanged() -> None:
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
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(
|
patch(
|
||||||
"agent.webapp.get_github_app_installation_token",
|
"agent.webapp.get_github_app_installation_token",
|
||||||
|
|
@ -352,7 +352,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None:
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
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.get_github_app_installation_token_with_expiry", get_token),
|
||||||
patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr),
|
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 (
|
with (
|
||||||
patch(
|
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.get_github_app_installation_token_with_expiry", get_token),
|
||||||
patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr),
|
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 (
|
with (
|
||||||
patch(
|
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(
|
patch(
|
||||||
"agent.webapp._get_thread_metadata_safe",
|
"agent.webapp._get_thread_metadata_safe",
|
||||||
new_callable=AsyncMock,
|
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),
|
patch("agent.webapp.set_reviewer_thread_metadata", side_effect=fake_set),
|
||||||
):
|
):
|
||||||
await webapp.process_github_pr_close(_pr_close_payload(action="closed"))
|
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
|
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))
|
captured.append((thread_id, kwargs))
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
|
||||||
"agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True
|
|
||||||
),
|
|
||||||
patch(
|
patch(
|
||||||
"agent.webapp._get_thread_metadata_safe",
|
"agent.webapp._get_thread_metadata_safe",
|
||||||
new_callable=AsyncMock,
|
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:
|
async def test_pr_close_skips_non_reviewer_threads() -> None:
|
||||||
fake_set = AsyncMock()
|
fake_set = AsyncMock()
|
||||||
with (
|
with (
|
||||||
patch(
|
|
||||||
"agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True
|
|
||||||
),
|
|
||||||
patch(
|
patch(
|
||||||
"agent.webapp._get_thread_metadata_safe",
|
"agent.webapp._get_thread_metadata_safe",
|
||||||
new_callable=AsyncMock,
|
new_callable=AsyncMock,
|
||||||
|
|
|
||||||
|
|
@ -733,12 +733,12 @@ export const api = {
|
||||||
request<NotionCredentialStatus>("/my-credentials/notion", {
|
request<NotionCredentialStatus>("/my-credentials/notion", {
|
||||||
method: "DELETE",
|
method: "DELETE",
|
||||||
}),
|
}),
|
||||||
listEnabledReviewRepos: () =>
|
listAutoReviewRepos: () =>
|
||||||
request<{ repos: Array<string> }>("/enabled-review-repos"),
|
request<{ repos: Array<string> }>("/enabled-review-repos"),
|
||||||
setEnabledReviewRepo: (full_name: string, enabled: boolean) =>
|
setAutoReviewRepo: (full_name: string, runAutomatically: boolean) =>
|
||||||
request<{ repos: Array<string> }>("/enabled-review-repos", {
|
request<{ repos: Array<string> }>("/enabled-review-repos", {
|
||||||
method: "PUT",
|
method: "PUT",
|
||||||
body: JSON.stringify({ full_name, enabled }),
|
body: JSON.stringify({ full_name, enabled: runAutomatically }),
|
||||||
}),
|
}),
|
||||||
usageLeaderboard: (period: UsageLeaderboardPeriod = "30d", limit = 10) =>
|
usageLeaderboard: (period: UsageLeaderboardPeriod = "30d", limit = 10) =>
|
||||||
request<UsageLeaderboardPayload>(
|
request<UsageLeaderboardPayload>(
|
||||||
|
|
|
||||||
|
|
@ -91,7 +91,7 @@ function ReviewPage() {
|
||||||
<AppShell
|
<AppShell
|
||||||
user={session.data}
|
user={session.data}
|
||||||
title="Open SWE Review"
|
title="Open SWE Review"
|
||||||
description="Automatically review pull requests for bugs and issues. Runs are billed based on underlying agent usage."
|
description="Review pull requests for bugs and issues on demand, or run reviews automatically. Runs are billed based on underlying agent usage."
|
||||||
>
|
>
|
||||||
<RepositoriesSection canEdit={canEdit} />
|
<RepositoriesSection canEdit={canEdit} />
|
||||||
|
|
||||||
|
|
@ -202,14 +202,14 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) {
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const enabled = useQuery({
|
const autoReview = useQuery({
|
||||||
queryKey: ["enabledReviewRepos"],
|
queryKey: ["autoReviewRepos"],
|
||||||
queryFn: api.listEnabledReviewRepos,
|
queryFn: api.listAutoReviewRepos,
|
||||||
});
|
});
|
||||||
|
|
||||||
const enabledSet = useMemo(
|
const autoReviewSet = useMemo(
|
||||||
() => new Set(enabled.data?.repos ?? []),
|
() => new Set(autoReview.data?.repos ?? []),
|
||||||
[enabled.data?.repos],
|
[autoReview.data?.repos],
|
||||||
);
|
);
|
||||||
|
|
||||||
const grouped = useMemo(() => {
|
const grouped = useMemo(() => {
|
||||||
|
|
@ -224,12 +224,12 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) {
|
||||||
return Array.from(byOwner.entries()).sort(([a], [b]) => a.localeCompare(b));
|
return Array.from(byOwner.entries()).sort(([a], [b]) => a.localeCompare(b));
|
||||||
}, [repos.data?.repositories]);
|
}, [repos.data?.repositories]);
|
||||||
|
|
||||||
const loading = repos.isLoading || enabled.isLoading;
|
const loading = repos.isLoading || autoReview.isLoading;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<SettingsSection
|
<SettingsSection
|
||||||
title="Repositories"
|
title="Repositories"
|
||||||
description="Source-control installations. Click into one to enable repos for automatic review."
|
description="All installed repositories support on-demand reviews. Click into an installation to configure automatic reviews."
|
||||||
>
|
>
|
||||||
<div className="divide-y divide-border">
|
<div className="divide-y divide-border">
|
||||||
{loading && (
|
{loading && (
|
||||||
|
|
@ -244,7 +244,7 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) {
|
||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
{grouped.map(([owner, list]) => {
|
{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 (
|
return (
|
||||||
<Link
|
<Link
|
||||||
key={owner}
|
key={owner}
|
||||||
|
|
@ -263,7 +263,7 @@ function RepositoriesSection({ canEdit: _canEdit }: { canEdit: boolean }) {
|
||||||
</div>
|
</div>
|
||||||
<div className="flex items-center gap-2 text-xs text-muted-foreground">
|
<div className="flex items-center gap-2 text-xs text-muted-foreground">
|
||||||
<span>
|
<span>
|
||||||
{enabledCount}/{list.length} Repositories Enabled
|
{autoReviewCount}/{list.length} Run Automatically
|
||||||
</span>
|
</span>
|
||||||
<CaretRightIcon className="size-3.5" />
|
<CaretRightIcon className="size-3.5" />
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -36,17 +36,17 @@ function RepositoriesOwnerPage() {
|
||||||
enabled: !!session.data,
|
enabled: !!session.data,
|
||||||
});
|
});
|
||||||
|
|
||||||
const enabled = useQuery({
|
const autoReview = useQuery({
|
||||||
queryKey: ["enabledReviewRepos"],
|
queryKey: ["autoReviewRepos"],
|
||||||
queryFn: api.listEnabledReviewRepos,
|
queryFn: api.listAutoReviewRepos,
|
||||||
enabled: !!session.data,
|
enabled: !!session.data,
|
||||||
});
|
});
|
||||||
|
|
||||||
const toggle = useMutation({
|
const toggleAutoReview = useMutation({
|
||||||
mutationFn: ({ full_name, on }: { full_name: string; on: boolean }) =>
|
mutationFn: ({ full_name, on }: { full_name: string; on: boolean }) =>
|
||||||
api.setEnabledReviewRepo(full_name, on),
|
api.setAutoReviewRepo(full_name, on),
|
||||||
onSuccess: (data) => {
|
onSuccess: (data) => {
|
||||||
qc.setQueryData(["enabledReviewRepos"], data);
|
qc.setQueryData(["autoReviewRepos"], data);
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -58,9 +58,9 @@ function RepositoriesOwnerPage() {
|
||||||
[repos.data?.repositories, owner],
|
[repos.data?.repositories, owner],
|
||||||
);
|
);
|
||||||
|
|
||||||
const enabledSet = useMemo(
|
const autoReviewSet = useMemo(
|
||||||
() => new Set(enabled.data?.repos ?? []),
|
() => new Set(autoReview.data?.repos ?? []),
|
||||||
[enabled.data?.repos],
|
[autoReview.data?.repos],
|
||||||
);
|
);
|
||||||
|
|
||||||
const [page, setPage] = useState(0);
|
const [page, setPage] = useState(0);
|
||||||
|
|
@ -82,8 +82,8 @@ function RepositoriesOwnerPage() {
|
||||||
if (!session.data) return <RequireLogin />;
|
if (!session.data) return <RequireLogin />;
|
||||||
|
|
||||||
const canEdit = session.data.is_admin;
|
const canEdit = session.data.is_admin;
|
||||||
const enabledCount = ownerRepos.filter((r) => enabledSet.has(r.full_name)).length;
|
const autoReviewCount = ownerRepos.filter((r) => autoReviewSet.has(r.full_name)).length;
|
||||||
const loading = repos.isLoading || enabled.isLoading;
|
const loading = repos.isLoading || autoReview.isLoading;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<AppShell
|
<AppShell
|
||||||
|
|
@ -91,8 +91,8 @@ function RepositoriesOwnerPage() {
|
||||||
title={owner}
|
title={owner}
|
||||||
description={
|
description={
|
||||||
canEdit
|
canEdit
|
||||||
? "Toggle a repository to opt it into automatic Open SWE Review."
|
? "Choose which repositories run Open SWE Review automatically. All installed repositories remain available for on-demand reviews."
|
||||||
: "Only team admins can modify enabled repositories."
|
: "Automatic review settings are read-only for non-admins."
|
||||||
}
|
}
|
||||||
backTo={{ to: "/review", label: "Back to Open SWE Review" }}
|
backTo={{ to: "/review", label: "Back to Open SWE Review" }}
|
||||||
>
|
>
|
||||||
|
|
@ -102,7 +102,7 @@ function RepositoriesOwnerPage() {
|
||||||
Repositories
|
Repositories
|
||||||
</h2>
|
</h2>
|
||||||
<span className="text-xs text-muted-foreground">
|
<span className="text-xs text-muted-foreground">
|
||||||
{enabledCount}/{ownerRepos.length} enabled
|
{autoReviewCount}/{ownerRepos.length} run automatically
|
||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
<div className="rounded-lg border border-border bg-card">
|
<div className="rounded-lg border border-border bg-card">
|
||||||
|
|
@ -118,7 +118,7 @@ function RepositoriesOwnerPage() {
|
||||||
)}
|
)}
|
||||||
<ul className="divide-y divide-border">
|
<ul className="divide-y divide-border">
|
||||||
{pageRepos.map((r) => {
|
{pageRepos.map((r) => {
|
||||||
const isEnabled = enabledSet.has(r.full_name);
|
const runsAutomatically = autoReviewSet.has(r.full_name);
|
||||||
return (
|
return (
|
||||||
<li
|
<li
|
||||||
key={r.full_name}
|
key={r.full_name}
|
||||||
|
|
@ -135,22 +135,26 @@ function RepositoriesOwnerPage() {
|
||||||
<span className="text-[10px] text-muted-foreground">private</span>
|
<span className="text-[10px] text-muted-foreground">private</span>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
<span
|
<div className="flex items-center gap-2">
|
||||||
title={
|
<span className="text-xs text-muted-foreground">Run automatically</span>
|
||||||
!canEdit
|
<span
|
||||||
? "Only team admins can modify enabled repositories"
|
title={
|
||||||
: undefined
|
!canEdit
|
||||||
}
|
? "Only team admins can modify automatic review settings"
|
||||||
className={!canEdit ? "cursor-not-allowed" : undefined}
|
: undefined
|
||||||
>
|
|
||||||
<Switch
|
|
||||||
checked={isEnabled}
|
|
||||||
disabled={!canEdit || toggle.isPending}
|
|
||||||
onCheckedChange={(v) =>
|
|
||||||
toggle.mutate({ full_name: r.full_name, on: v })
|
|
||||||
}
|
}
|
||||||
/>
|
className={!canEdit ? "cursor-not-allowed" : undefined}
|
||||||
</span>
|
>
|
||||||
|
<Switch
|
||||||
|
aria-label={`Run reviews automatically for ${r.full_name}`}
|
||||||
|
checked={runsAutomatically}
|
||||||
|
disabled={!canEdit || toggleAutoReview.isPending}
|
||||||
|
onCheckedChange={(v) =>
|
||||||
|
toggleAutoReview.mutate({ full_name: r.full_name, on: v })
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
</li>
|
</li>
|
||||||
);
|
);
|
||||||
})}
|
})}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue