mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 10:23:14 +00:00
feat: activate PR babysitting UI toggles for autofix and trigger mode (#1561)
* feat: activate PR babysitting UI toggles for autofix and trigger mode Remove the "coming soon" gating on the Autofix Mode, Autofix Severity Threshold, and Trigger Mode controls in the review settings page so admins can enable CI auto-fix and review-comment resolution on PRs that Open SWE opens. The backend (ci_autofix.py, webapp.py webhook routing) was already fully wired — only the UI was disabled. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: simplify autofix to on/off toggle, remove severity threshold Replace the four-level AutofixMode (off/low/medium/high) and the autofix_severity_threshold setting with a single boolean autofix_enabled toggle. The severity threshold was leftover from the reviewer finding-severity model and does not apply to CI autofix; the agent should fix any failing CI and resolve any comments on PRs it opens. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: move autofix toggle to per-user profile, remove team-level setting The autofix toggle is now per-user (auto_fix_ci in the user profile) instead of team-level (admin-only). This uses the existing auto_fix_ci field that was already in ProfileUpdate but never wired up. Changes: - ci_autofix.py: check per-user auto_fix_ci profile flag after resolving the agent thread's github_login, instead of checking team-level autofix_enabled before knowing the PR - webapp.py: removed early is_autofix_enabled() webhook gates; the per-user check now happens in ci_autofix.py once the thread is found - team_settings.py: removed autofix_enabled field, is_autofix_enabled() - cloud-agents.tsx: enabled the auto_fix_ci toggle (was comingSoon) - review.tsx: removed the admin-level autofix switch - Updated tests and AGENTS.md The agent graph (not the reviewer) is what gets dispatched - this was already correct in ci_autofix.py line 223: client.runs.create( thread_id, "agent", ...). Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: batch PR babysitting events Remove the leftover trigger-mode gate from PR babysitting and batch new CI/review events while an agent run is already active so the running agent can handle the latest PR state before finishing. Also moves review-feedback permission checks behind the per-user opt-out and applies the auto-fix profile gate to merge-conflict babysitting. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: consume batched babysitting events Teach the agent queue middleware to turn pending PR babysitting metadata into an injected instruction for the active run, so batched CI/review events are not dropped while still avoiding duplicate run creation. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: address review findings in PR babysitting batching - Route batched events through the LangGraph store (read in-process by the message-queue middleware) instead of a per-model-call threads.get on every agent thread. - Only record an attempt / mark the head SHA handled on a real dispatch, not on a batch, so an event isn't permanently dropped if the in-flight run ends before consuming it. - Carry the reviewer's comment through batched review feedback instead of replacing it with a generic re-check nudge. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Johannes du Plessis
GitHub
parent
60b7f4677a
commit
8e39f62122
13 changed files with 312 additions and 382 deletions
|
|
@ -34,7 +34,7 @@ make format # ruff format + ruff check --fix
|
|||
|
||||
The FastAPI app is `agent.webapp:app`.
|
||||
|
||||
CI auto-fix ("PR babysitting") lives in `agent/ci_autofix.py`: when a CI check fails (webhook `check_run` / `check_suite` / `workflow_run` / `status`) or a reviewer leaves actionable feedback on a PR Open SWE opened, it locates the originating agent thread (by `pr_url` metadata) and dispatches a confidence-gated fix run on the `agent` graph. Gated by the team `autofix_mode` / `trigger_mode` settings, the enabled-repos opt-in, and a per-PR `@open-swe autofix on|off` toggle (`agent/dashboard/autofix_state.py`). Skip-rules (base-branch failures, human commits, dedupe, loop cap) all live in `ci_autofix.py`.
|
||||
CI auto-fix ("PR babysitting") lives in `agent/ci_autofix.py`: when a CI check fails (webhook `check_run` / `check_suite` / `workflow_run` / `status`) or a reviewer leaves actionable feedback on a PR Open SWE opened, it locates the originating agent thread (by `pr_url` metadata) and dispatches a confidence-gated fix run on the `agent` graph. Gated by the per-user `auto_fix_ci` profile flag, the enabled-repos opt-in, and a per-PR `@open-swe autofix on|off` toggle (`agent/dashboard/autofix_state.py`). Skip-rules (base-branch failures, human commits, same-head dedupe, batching while runs are active, loop cap) all live in `ci_autofix.py`.
|
||||
|
||||
## Architecture
|
||||
|
||||
|
|
|
|||
|
|
@ -11,8 +11,8 @@ loop-capping live in one place. Skip-rules mirror Cursor/Claude Code:
|
|||
* Only PRs Open SWE authored (an agent thread with this ``pr_url`` exists).
|
||||
* Skip failures inherited from the base branch.
|
||||
* Skip when the latest commit was authored by a human (don't fight pushes).
|
||||
* Dedupe per (head SHA + failing-check set); cap total attempts.
|
||||
* Honor team ``autofix_mode`` / ``trigger_mode`` and the per-PR opt-out.
|
||||
* Dedupe per head SHA; cap total attempts.
|
||||
* Honor the per-user ``auto_fix_ci`` profile flag and the per-PR opt-out.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -22,9 +22,9 @@ from typing import Any
|
|||
|
||||
from langgraph_sdk import get_client
|
||||
|
||||
from .dashboard.agent_overrides import load_profile, resolve_login_from_email_async
|
||||
from .dashboard.autofix_state import is_pr_autofix_disabled
|
||||
from .dashboard.enabled_repos import is_review_repo_enabled
|
||||
from .dashboard.team_settings import get_autofix_settings
|
||||
from .reviewer_findings import REVIEWER_THREAD_KIND
|
||||
from .utils.dashboard_links import dashboard_thread_url
|
||||
from .utils.github_app import get_github_app_installation_token
|
||||
|
|
@ -32,6 +32,7 @@ from .utils.github_checks import post_autofix_status_check
|
|||
from .utils.github_ci import (
|
||||
fetch_open_pr_for_branch,
|
||||
fetch_pr,
|
||||
has_repo_write_permission,
|
||||
head_commit_author_login,
|
||||
list_failing_check_runs,
|
||||
list_failing_statuses,
|
||||
|
|
@ -41,7 +42,6 @@ from .utils.github_org_membership import INTERNAL_BOT_LOGINS
|
|||
from .utils.thread_ops import (
|
||||
is_thread_active,
|
||||
langgraph_client,
|
||||
queue_message_for_thread,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -51,10 +51,27 @@ logger = logging.getLogger(__name__)
|
|||
MAX_AUTOFIX_ATTEMPTS = 10
|
||||
# Keep the dedupe list bounded on thread metadata.
|
||||
_MAX_HANDLED_KEYS = 30
|
||||
# Store location for batched auto-fix events, consumed by the message-queue middleware.
|
||||
_PENDING_AUTOFIX_NS = "autofix"
|
||||
_PENDING_AUTOFIX_KEY = "pending_event"
|
||||
|
||||
|
||||
def _dedupe_key(head_sha: str, failing_names: list[str]) -> str:
|
||||
return f"{head_sha}:" + ",".join(sorted(failing_names))
|
||||
def _dedupe_key(head_sha: str) -> str:
|
||||
return head_sha
|
||||
|
||||
|
||||
async def _user_autofix_enabled(github_login: str, user_email: str = "") -> bool:
|
||||
"""Check the per-user ``auto_fix_ci`` profile flag (defaults to True)."""
|
||||
login = github_login.strip() if isinstance(github_login, str) else ""
|
||||
if not login and user_email:
|
||||
login = await resolve_login_from_email_async(user_email)
|
||||
if not login:
|
||||
return True
|
||||
profile = await load_profile(login)
|
||||
if not isinstance(profile, dict):
|
||||
return True
|
||||
value = profile.get("auto_fix_ci")
|
||||
return value if isinstance(value, bool) else True
|
||||
|
||||
|
||||
async def find_agent_thread_for_pr(pr_url: str) -> tuple[str, dict[str, Any]] | None:
|
||||
|
|
@ -124,7 +141,10 @@ def _build_ci_fix_prompt(
|
|||
"need, then stop.\n"
|
||||
"4. Never force-push. Never weaken or delete test assertions just to go "
|
||||
"green unless the behavior change is intentional and correct.\n"
|
||||
"5. After you push, CI re-runs automatically — you don't need to merge."
|
||||
"5. Before finishing, re-check the PR's latest CI status and review "
|
||||
"comments. Address any newly failed checks or unhandled actionable "
|
||||
"comments that arrived while you were working.\n"
|
||||
"6. After you push, CI re-runs automatically — you don't need to merge."
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -149,19 +169,24 @@ def _build_review_feedback_prompt(
|
|||
"existing branch.\n"
|
||||
"2. If the comment is ambiguous, opinion-based, or needs a design "
|
||||
"decision, reply on the PR asking for clarification instead of guessing.\n"
|
||||
"3. Never force-push. Reply to the reviewer on GitHub to explain what "
|
||||
"3. Before finishing, re-check the PR's latest review comments and CI "
|
||||
"status. Address any newly arrived actionable comments or failed checks "
|
||||
"that are clear and deterministic.\n"
|
||||
"4. Never force-push. Reply to the reviewer on GitHub to explain what "
|
||||
"you changed."
|
||||
)
|
||||
|
||||
|
||||
async def _thread_autofix_state(metadata: dict[str, Any]) -> tuple[int, list[str], str]:
|
||||
async def _thread_autofix_state(metadata: dict[str, Any]) -> tuple[int, list[str], str, str]:
|
||||
attempts = metadata.get("autofix_attempts")
|
||||
attempts = attempts if isinstance(attempts, int) and attempts >= 0 else 0
|
||||
handled = metadata.get("autofix_handled")
|
||||
handled = [h for h in handled if isinstance(h, str)] if isinstance(handled, list) else []
|
||||
github_login = metadata.get("github_login")
|
||||
github_login = github_login if isinstance(github_login, str) else ""
|
||||
return attempts, handled, github_login
|
||||
user_email = metadata.get("triggering_user_email")
|
||||
user_email = user_email if isinstance(user_email, str) else ""
|
||||
return attempts, handled, github_login, user_email
|
||||
|
||||
|
||||
async def _record_attempt(
|
||||
|
|
@ -212,11 +237,44 @@ def _run_configurable(
|
|||
return configurable
|
||||
|
||||
|
||||
async def _dispatch_or_queue(thread_id: str, prompt: str, *, configurable: dict[str, Any]) -> str:
|
||||
async def _mark_pending_autofix_event(thread_id: str, reason: str, detail: str = "") -> None:
|
||||
"""Record a batched auto-fix event in the store the message-queue middleware reads.
|
||||
|
||||
Uses the same store namespace mechanism as ``queue_message_for_thread`` so the
|
||||
in-flight run picks it up in-process at its next ``before_model`` step — no
|
||||
per-step thread fetch. ``detail`` (e.g. a reviewer's comment) is accumulated so
|
||||
specifics aren't lost when several events batch against one busy run.
|
||||
"""
|
||||
client = langgraph_client()
|
||||
namespace = (_PENDING_AUTOFIX_NS, thread_id)
|
||||
try:
|
||||
details: list[str] = []
|
||||
try:
|
||||
existing = await client.store.get_item(namespace, _PENDING_AUTOFIX_KEY)
|
||||
if existing and existing.get("value"):
|
||||
prior = existing["value"].get("details")
|
||||
if isinstance(prior, list):
|
||||
details = [d for d in prior if isinstance(d, str)]
|
||||
except Exception: # noqa: BLE001
|
||||
logger.debug("No existing pending auto-fix event for thread %s", thread_id)
|
||||
if detail and detail not in details:
|
||||
details.append(detail)
|
||||
await client.store.put_item(
|
||||
namespace, _PENDING_AUTOFIX_KEY, {"reason": reason, "details": details}
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
logger.debug(
|
||||
"Failed to record pending auto-fix event for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
|
||||
|
||||
async def _dispatch_or_batch(
|
||||
thread_id: str, prompt: str, *, configurable: dict[str, Any], reason: str, detail: str = ""
|
||||
) -> str:
|
||||
if await is_thread_active(thread_id):
|
||||
logger.info("Agent thread %s busy; queuing auto-fix message", thread_id)
|
||||
await queue_message_for_thread(thread_id, prompt)
|
||||
return "queued"
|
||||
logger.info("Agent thread %s busy; batching auto-fix event %s", thread_id, reason)
|
||||
await _mark_pending_autofix_event(thread_id, reason, detail)
|
||||
return "batched"
|
||||
client = langgraph_client()
|
||||
await client.runs.create(
|
||||
thread_id,
|
||||
|
|
@ -247,9 +305,6 @@ async def handle_ci_failure(
|
|||
if not owner or not repo:
|
||||
return "missing_repo"
|
||||
|
||||
settings = await get_autofix_settings()
|
||||
if settings["autofix_mode"] == "off":
|
||||
return "autofix_disabled_team"
|
||||
if not await is_review_repo_enabled(owner, repo):
|
||||
return "repo_not_enabled"
|
||||
|
||||
|
|
@ -284,12 +339,11 @@ async def handle_ci_failure(
|
|||
return "no_agent_thread"
|
||||
thread_id, metadata = found
|
||||
|
||||
attempts, handled, github_login = await _thread_autofix_state(metadata)
|
||||
attempts, handled, github_login, user_email = await _thread_autofix_state(metadata)
|
||||
|
||||
if not await _user_autofix_enabled(github_login, user_email):
|
||||
return "autofix_disabled_user"
|
||||
|
||||
if settings["trigger_mode"] == "manual":
|
||||
return "trigger_manual"
|
||||
if settings["trigger_mode"] == "once_per_pr" and attempts >= 1:
|
||||
return "once_per_pr_done"
|
||||
if attempts >= MAX_AUTOFIX_ATTEMPTS:
|
||||
await post_autofix_status_check(
|
||||
owner=owner,
|
||||
|
|
@ -321,8 +375,7 @@ async def handle_ci_failure(
|
|||
if not actionable:
|
||||
return "all_failing_on_base"
|
||||
|
||||
failing_names = [c.get("name", "") for c in actionable]
|
||||
dedupe_key = _dedupe_key(head_sha, failing_names)
|
||||
dedupe_key = _dedupe_key(head_sha)
|
||||
if dedupe_key in handled:
|
||||
return "already_handled"
|
||||
|
||||
|
|
@ -351,28 +404,34 @@ async def handle_ci_failure(
|
|||
head_sha=head_sha,
|
||||
failing_checks=actionable,
|
||||
)
|
||||
result = await _dispatch_or_queue(
|
||||
result = await _dispatch_or_batch(
|
||||
thread_id,
|
||||
prompt,
|
||||
configurable=_run_configurable(
|
||||
metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number
|
||||
),
|
||||
reason="ci_failure",
|
||||
)
|
||||
await _record_attempt(
|
||||
thread_id, attempts=attempts, handled=handled, dedupe_key=dedupe_key, head_sha=head_sha
|
||||
)
|
||||
await post_autofix_status_check(
|
||||
owner=owner,
|
||||
repo=repo,
|
||||
head_sha=head_sha,
|
||||
token=token,
|
||||
title=f"Auto-fixing {len(actionable)} failing check(s)",
|
||||
summary=(
|
||||
"Open SWE is investigating the failing checks and will push a fix if "
|
||||
"the cause is clear. Track progress in the linked run."
|
||||
),
|
||||
details_url=dashboard_thread_url(thread_id),
|
||||
)
|
||||
if result == "dispatched":
|
||||
# Only burn an attempt / mark the SHA handled on a real dispatch. A batched
|
||||
# event is just a nudge to the in-flight run; if that run ends before
|
||||
# consuming it, leaving the SHA un-handled lets a later webhook or the sweep
|
||||
# re-dispatch instead of silently dropping the failure.
|
||||
await _record_attempt(
|
||||
thread_id, attempts=attempts, handled=handled, dedupe_key=dedupe_key, head_sha=head_sha
|
||||
)
|
||||
await post_autofix_status_check(
|
||||
owner=owner,
|
||||
repo=repo,
|
||||
head_sha=head_sha,
|
||||
token=token,
|
||||
title=f"Auto-fixing {len(actionable)} failing check(s)",
|
||||
summary=(
|
||||
"Open SWE is investigating the failing checks and will push a fix if "
|
||||
"the cause is clear. Track progress in the linked run."
|
||||
),
|
||||
details_url=dashboard_thread_url(thread_id),
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
|
|
@ -392,11 +451,6 @@ async def handle_review_feedback(
|
|||
if not owner or not repo or not pr_url:
|
||||
return "missing_repo"
|
||||
|
||||
settings = await get_autofix_settings()
|
||||
if settings["autofix_mode"] == "off":
|
||||
return "autofix_disabled_team"
|
||||
if settings["trigger_mode"] == "manual":
|
||||
return "trigger_manual"
|
||||
if not await is_review_repo_enabled(owner, repo):
|
||||
return "repo_not_enabled"
|
||||
if await is_pr_autofix_disabled(owner, repo, pr_number):
|
||||
|
|
@ -407,6 +461,24 @@ async def handle_review_feedback(
|
|||
return "no_agent_thread"
|
||||
thread_id, metadata = found
|
||||
|
||||
_, _, github_login, user_email = await _thread_autofix_state(metadata)
|
||||
if not await _user_autofix_enabled(github_login, user_email):
|
||||
return "autofix_disabled_user"
|
||||
|
||||
if token is None:
|
||||
token = await get_github_app_installation_token()
|
||||
if not token:
|
||||
return "no_token"
|
||||
if not await has_repo_write_permission(owner=owner, repo=repo, username=reviewer, token=token):
|
||||
logger.info(
|
||||
"Skipping auto-fix review feedback on %s/%s#%s: %s lacks write access",
|
||||
owner,
|
||||
repo,
|
||||
pr_number,
|
||||
reviewer or "<unknown>",
|
||||
)
|
||||
return "reviewer_no_write_permission"
|
||||
|
||||
prompt = _build_review_feedback_prompt(
|
||||
owner=owner,
|
||||
repo=repo,
|
||||
|
|
@ -415,12 +487,16 @@ async def handle_review_feedback(
|
|||
reviewer=reviewer,
|
||||
body=body,
|
||||
)
|
||||
return await _dispatch_or_queue(
|
||||
return await _dispatch_or_batch(
|
||||
thread_id,
|
||||
prompt,
|
||||
configurable=_run_configurable(
|
||||
metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number
|
||||
),
|
||||
reason="review_feedback",
|
||||
detail=f"Reviewer {reviewer or 'unknown'} commented: {body.strip()}"
|
||||
if body.strip()
|
||||
else "",
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -431,7 +507,7 @@ async def sweep_open_prs() -> dict[str, int]:
|
|||
only path that can react to base-branch merge conflicts (GitHub emits no
|
||||
webhook for those).
|
||||
"""
|
||||
counts = {"scanned": 0, "dispatched": 0, "queued": 0, "conflicts": 0}
|
||||
counts = {"scanned": 0, "dispatched": 0, "batched": 0, "conflicts": 0}
|
||||
token = await get_github_app_installation_token()
|
||||
if not token:
|
||||
logger.warning("CI monitor sweep: no GitHub App token")
|
||||
|
|
@ -487,8 +563,8 @@ async def sweep_open_prs() -> dict[str, int]:
|
|||
)
|
||||
if result == "dispatched":
|
||||
counts["dispatched"] += 1
|
||||
elif result == "queued":
|
||||
counts["queued"] += 1
|
||||
elif result == "batched":
|
||||
counts["batched"] += 1
|
||||
logger.info("CI monitor sweep complete: %s", counts)
|
||||
return counts
|
||||
|
||||
|
|
@ -503,6 +579,9 @@ async def _flag_merge_conflict(
|
|||
if found is None:
|
||||
return
|
||||
thread_id, metadata = found
|
||||
_, _, github_login, user_email = await _thread_autofix_state(metadata)
|
||||
if not await _user_autofix_enabled(github_login, user_email):
|
||||
return
|
||||
if metadata.get("autofix_conflict_head") == head_sha:
|
||||
return
|
||||
prompt = (
|
||||
|
|
@ -512,12 +591,13 @@ async def _flag_merge_conflict(
|
|||
"conflict resolution is ambiguous, comment on the PR and ask before "
|
||||
"guessing. Never force-push over commits already on the remote."
|
||||
)
|
||||
await _dispatch_or_queue(
|
||||
await _dispatch_or_batch(
|
||||
thread_id,
|
||||
prompt,
|
||||
configurable=_run_configurable(
|
||||
metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number
|
||||
),
|
||||
reason="merge_conflict",
|
||||
)
|
||||
try:
|
||||
await get_client().threads.update(
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
"""Per-PR auto-fix opt-out, stored in the LangGraph Store.
|
||||
|
||||
Team-wide auto-fix is gated by :func:`agent.dashboard.team_settings.is_autofix_enabled`.
|
||||
Auto-fix is gated by the per-user ``auto_fix_ci`` profile flag.
|
||||
On top of that, a single PR can be silenced with ``@open-swe autofix off`` (and
|
||||
re-enabled with ``@open-swe autofix on``), mirroring Cursor's
|
||||
``@cursor autofix off`` per-PR control. The toggle lives here rather than on the
|
||||
|
|
|
|||
|
|
@ -27,21 +27,15 @@ logger = logging.getLogger(__name__)
|
|||
TEAM_SETTINGS_NAMESPACE: list[str] = ["team_settings"]
|
||||
TEAM_SETTINGS_KEY = "default"
|
||||
|
||||
TriggerMode = Literal["every_push", "once_per_pr", "manual"]
|
||||
AutofixMode = Literal["off", "low", "medium", "high"]
|
||||
|
||||
# Cap the org-wide guidelines so a runaway value can't dominate the reviewer
|
||||
# prompt. Generous enough for a detailed policy, small enough to stay bounded.
|
||||
ORG_GUIDELINES_MAX_CHARS = 10_000
|
||||
|
||||
|
||||
class TeamSettingsUpdate(BaseModel):
|
||||
trigger_mode: TriggerMode = "every_push"
|
||||
review_draft_prs: bool = False
|
||||
pr_summaries: bool = True
|
||||
review_trace_links: bool = True
|
||||
autofix_mode: AutofixMode = "off"
|
||||
autofix_severity_threshold: AutofixMode = "medium"
|
||||
org_guidelines: str | None = None
|
||||
default_agent_model: str | None = None
|
||||
default_agent_reasoning_effort: str | None = None
|
||||
|
|
@ -135,12 +129,9 @@ def _parse_repo(value: object) -> dict[str, str] | None:
|
|||
def _default_settings() -> dict[str, Any]:
|
||||
fallback_model, fallback_effort = default_model_pair()
|
||||
return {
|
||||
"trigger_mode": "every_push",
|
||||
"review_draft_prs": False,
|
||||
"pr_summaries": True,
|
||||
"review_trace_links": True,
|
||||
"autofix_mode": "off",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"org_guidelines": None,
|
||||
"default_agent_model": fallback_model,
|
||||
"default_agent_reasoning_effort": fallback_effort,
|
||||
|
|
@ -178,21 +169,21 @@ async def get_team_settings() -> dict[str, Any]:
|
|||
# selection) still surface the hardcoded default instead of a null.
|
||||
overlay = {k: v for k, v in value.items() if v is not None}
|
||||
merged = {**defaults, **overlay}
|
||||
# Drop obsolete trigger mode values so a legacy record doesn't surface a
|
||||
# value the new TriggerMode literal would reject on the next PUT.
|
||||
if merged.get("trigger_mode") not in {"every_push", "once_per_pr", "manual"}:
|
||||
merged["trigger_mode"] = defaults["trigger_mode"]
|
||||
for stale_field in (
|
||||
"trigger_mode",
|
||||
"autofix_mode",
|
||||
"autofix_severity_threshold",
|
||||
"autofix_enabled",
|
||||
):
|
||||
merged.pop(stale_field, None)
|
||||
return merged
|
||||
|
||||
|
||||
async def upsert_team_settings(update: TeamSettingsUpdate) -> dict[str, Any]:
|
||||
value: dict[str, Any] = {
|
||||
"trigger_mode": update.trigger_mode,
|
||||
"review_draft_prs": update.review_draft_prs,
|
||||
"pr_summaries": update.pr_summaries,
|
||||
"review_trace_links": update.review_trace_links,
|
||||
"autofix_mode": update.autofix_mode,
|
||||
"autofix_severity_threshold": update.autofix_severity_threshold,
|
||||
"org_guidelines": update.org_guidelines,
|
||||
"default_agent_model": update.default_agent_model,
|
||||
"default_agent_reasoning_effort": update.default_agent_reasoning_effort,
|
||||
|
|
@ -306,31 +297,6 @@ async def get_team_default_grouping_model() -> tuple[str, str]:
|
|||
)
|
||||
|
||||
|
||||
async def get_autofix_settings() -> dict[str, Any]:
|
||||
"""Return the team-wide auto-fix config: mode, severity threshold, trigger mode."""
|
||||
settings = await get_team_settings()
|
||||
mode = settings.get("autofix_mode")
|
||||
if mode not in {"off", "low", "medium", "high"}:
|
||||
mode = "off"
|
||||
threshold = settings.get("autofix_severity_threshold")
|
||||
if threshold not in {"off", "low", "medium", "high"}:
|
||||
threshold = "medium"
|
||||
trigger = settings.get("trigger_mode")
|
||||
if trigger not in {"every_push", "once_per_pr", "manual"}:
|
||||
trigger = "every_push"
|
||||
return {
|
||||
"autofix_mode": mode,
|
||||
"autofix_severity_threshold": threshold,
|
||||
"trigger_mode": trigger,
|
||||
}
|
||||
|
||||
|
||||
async def is_autofix_enabled() -> bool:
|
||||
"""Return whether team-wide auto-fix is turned on (mode != ``off``)."""
|
||||
settings = await get_autofix_settings()
|
||||
return settings["autofix_mode"] != "off"
|
||||
|
||||
|
||||
async def get_team_review_trace_links_enabled() -> bool:
|
||||
"""Return whether GitHub review bodies should include a LangSmith trace link."""
|
||||
settings = await get_team_settings()
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import httpx
|
|||
from langchain.agents.middleware import AgentState, before_model
|
||||
from langgraph.config import get_config, get_store
|
||||
from langgraph.runtime import Runtime
|
||||
from langgraph.store.base import BaseStore
|
||||
from langgraph_sdk import get_client
|
||||
|
||||
from ..dashboard.options import model_supports_images
|
||||
|
|
@ -91,6 +92,49 @@ def _is_dashboard_queued_message(content: object) -> bool:
|
|||
return isinstance(content, dict) and content.get("source") == "dashboard"
|
||||
|
||||
|
||||
def _message_update(content_blocks: list[dict[str, Any]], thread_id: str) -> dict[str, Any] | None:
|
||||
if not content_blocks:
|
||||
return None
|
||||
logger.info(
|
||||
"Injected %d queued message block(s) into state for thread %s",
|
||||
len(content_blocks),
|
||||
thread_id,
|
||||
)
|
||||
return {"messages": [{"role": "user", "content": content_blocks}]}
|
||||
|
||||
|
||||
async def _consume_pending_autofix_event(store: BaseStore, thread_id: str) -> str | None:
|
||||
"""Pull and clear a batched PR-babysitting event from the store (no thread fetch)."""
|
||||
namespace = ("autofix", thread_id)
|
||||
try:
|
||||
item = await store.aget(namespace, "pending_event")
|
||||
except Exception: # noqa: BLE001
|
||||
logger.debug(
|
||||
"Could not read pending auto-fix event for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
return None
|
||||
if item is None or not item.value.get("reason"):
|
||||
return None
|
||||
try:
|
||||
await store.adelete(namespace, "pending_event")
|
||||
except Exception: # noqa: BLE001
|
||||
logger.debug(
|
||||
"Could not clear pending auto-fix event for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
message = (
|
||||
"A PR babysitting event arrived while you were already working on this PR. "
|
||||
"Do not start a separate run for that event. Before finishing, re-check the "
|
||||
"PR's latest CI status and review comments, then address any newly failed "
|
||||
"checks or actionable comments that are clear and deterministic."
|
||||
)
|
||||
details = item.value.get("details")
|
||||
if isinstance(details, list):
|
||||
joined = "\n\n".join(d for d in details if isinstance(d, str) and d)
|
||||
if joined:
|
||||
message += "\n\nNewly arrived feedback to address:\n" + joined
|
||||
return message
|
||||
|
||||
|
||||
@before_model(state_schema=LinearNotifyState)
|
||||
async def check_message_queue_before_model( # noqa: PLR0911
|
||||
state: LinearNotifyState, # noqa: ARG001
|
||||
|
|
@ -122,16 +166,21 @@ async def check_message_queue_before_model( # noqa: PLR0911
|
|||
if store is None:
|
||||
return None
|
||||
|
||||
content_blocks: list[dict[str, Any]] = []
|
||||
pending_autofix = await _consume_pending_autofix_event(store, thread_id)
|
||||
if pending_autofix:
|
||||
content_blocks.append({"type": "text", "text": pending_autofix})
|
||||
|
||||
namespace = ("queue", thread_id)
|
||||
|
||||
try:
|
||||
queued_item = await store.aget(namespace, "pending_messages")
|
||||
except Exception as e: # noqa: BLE001
|
||||
logger.warning("Failed to get queued item: %s", e)
|
||||
return None
|
||||
return _message_update(content_blocks, thread_id)
|
||||
|
||||
if queued_item is None:
|
||||
return None
|
||||
return _message_update(content_blocks, thread_id)
|
||||
|
||||
queued_value = queued_item.value
|
||||
queued_messages = queued_value.get("messages", [])
|
||||
|
|
@ -140,7 +189,7 @@ async def check_message_queue_before_model( # noqa: PLR0911
|
|||
await store.adelete(namespace, "pending_messages")
|
||||
|
||||
if not queued_messages:
|
||||
return None
|
||||
return _message_update(content_blocks, thread_id)
|
||||
|
||||
logger.info(
|
||||
"Found %d queued message(s) for thread %s, injecting into state",
|
||||
|
|
@ -157,7 +206,6 @@ async def check_message_queue_before_model( # noqa: PLR0911
|
|||
if has_images:
|
||||
resolved_model_id = await _resolve_thread_model_id(thread_id)
|
||||
|
||||
content_blocks: list[dict[str, Any]] = []
|
||||
for msg in queued_messages:
|
||||
content = msg.get("content")
|
||||
if _is_dashboard_queued_message(content):
|
||||
|
|
@ -177,21 +225,7 @@ async def check_message_queue_before_model( # noqa: PLR0911
|
|||
logger.debug("Queued message contains text content")
|
||||
content_blocks.append({"type": "text", "text": content})
|
||||
|
||||
if not content_blocks:
|
||||
return None
|
||||
|
||||
new_message = {
|
||||
"role": "user",
|
||||
"content": content_blocks,
|
||||
}
|
||||
|
||||
logger.info(
|
||||
"Injected %d queued message(s) into state for thread %s",
|
||||
len(content_blocks),
|
||||
thread_id,
|
||||
)
|
||||
|
||||
return {"messages": [new_message]} # noqa: TRY300
|
||||
return _message_update(content_blocks, thread_id) # noqa: TRY300
|
||||
except Exception:
|
||||
logger.exception("Error in check_message_queue_before_model")
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -35,7 +35,6 @@ from .dashboard.profiles import get_profile, get_valid_access_token, has_access_
|
|||
from .dashboard.team_settings import (
|
||||
get_team_default_repo,
|
||||
get_team_settings,
|
||||
is_autofix_enabled,
|
||||
)
|
||||
from .dashboard.user_mappings import (
|
||||
email_for_login,
|
||||
|
|
@ -72,7 +71,6 @@ from .utils.github_app import (
|
|||
from .utils.github_checks import complete_review_check_run, create_review_check_run
|
||||
from .utils.github_ci import (
|
||||
branch_from_check_payload,
|
||||
has_repo_write_permission,
|
||||
head_sha_from_check_payload,
|
||||
is_failing_ci_payload,
|
||||
)
|
||||
|
|
@ -2752,20 +2750,6 @@ async def process_github_autofix_review(payload: dict[str, Any], event_type: str
|
|||
body = (comment.get("body") or "") if isinstance(comment, dict) else ""
|
||||
if not body.strip() or reviewer in INTERNAL_BOT_LOGINS:
|
||||
return
|
||||
# Defense-in-depth beyond the author_association gate: confirm the reviewer
|
||||
# actually has write access before dispatching a write-capable agent run.
|
||||
token = await get_github_app_installation_token()
|
||||
if not token or not await has_repo_write_permission(
|
||||
owner=ref["owner"], repo=ref["name"], username=reviewer, token=token
|
||||
):
|
||||
logger.info(
|
||||
"Skipping auto-fix review feedback on %s/%s#%s: %s lacks write access",
|
||||
ref["owner"],
|
||||
ref["name"],
|
||||
ref["number"],
|
||||
reviewer or "<unknown>",
|
||||
)
|
||||
return
|
||||
result = await handle_review_feedback(
|
||||
repo_config={"owner": ref["owner"], "name": ref["name"]},
|
||||
pr_number=ref["number"],
|
||||
|
|
@ -3348,10 +3332,10 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
|||
return {"status": "accepted", "message": "Processing GitHub push for reviewer watch"}
|
||||
|
||||
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_autofix_enabled():
|
||||
return {"status": "ignored", "reason": "Auto-fix is disabled"}
|
||||
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"}
|
||||
|
|
@ -3432,12 +3416,11 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
|||
if _is_actionable_review_payload(payload, event_type) and await _is_repo_enabled_for_review(
|
||||
webhook_repo_config
|
||||
):
|
||||
if await is_autofix_enabled():
|
||||
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
||||
if gate_rejection is not None:
|
||||
return gate_rejection
|
||||
background_tasks.add_task(process_github_autofix_review, payload, event_type)
|
||||
return {"status": "accepted", "message": "Processing auto-fix review feedback"}
|
||||
gate_rejection = await _enforce_public_repo_org_gate(payload, event_type)
|
||||
if gate_rejection is not None:
|
||||
return gate_rejection
|
||||
background_tasks.add_task(process_github_autofix_review, payload, event_type)
|
||||
return {"status": "accepted", "message": "Processing auto-fix review feedback"}
|
||||
logger.debug(
|
||||
"Ignoring GitHub %s%s that does not mention @openswe or @open-swe",
|
||||
event_type,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
"""Unit tests for per-PR auto-fix opt-out state and team settings accessor."""
|
||||
"""Unit tests for per-PR auto-fix opt-out state."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
|
@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
|||
|
||||
import pytest
|
||||
|
||||
from agent.dashboard import autofix_state, team_settings
|
||||
from agent.dashboard import autofix_state
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -31,40 +31,3 @@ async def test_set_and_check_pr_disabled() -> None:
|
|||
assert await autofix_state.is_pr_autofix_disabled("o", "r", 5) is True
|
||||
await autofix_state.set_pr_autofix_disabled("o", "r", 5, False)
|
||||
assert await autofix_state.is_pr_autofix_disabled("o", "r", 5) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_autofix_settings_normalizes() -> None:
|
||||
with patch.object(
|
||||
team_settings,
|
||||
"get_team_settings",
|
||||
AsyncMock(
|
||||
return_value={
|
||||
"autofix_mode": "bogus",
|
||||
"autofix_severity_threshold": "high",
|
||||
"trigger_mode": "weird",
|
||||
}
|
||||
),
|
||||
):
|
||||
settings = await team_settings.get_autofix_settings()
|
||||
assert settings == {
|
||||
"autofix_mode": "off",
|
||||
"autofix_severity_threshold": "high",
|
||||
"trigger_mode": "every_push",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_is_autofix_enabled() -> None:
|
||||
with patch.object(
|
||||
team_settings,
|
||||
"get_team_settings",
|
||||
AsyncMock(return_value={"autofix_mode": "high"}),
|
||||
):
|
||||
assert await team_settings.is_autofix_enabled() is True
|
||||
with patch.object(
|
||||
team_settings,
|
||||
"get_team_settings",
|
||||
AsyncMock(return_value={"autofix_mode": "off"}),
|
||||
):
|
||||
assert await team_settings.is_autofix_enabled() is False
|
||||
|
|
|
|||
|
|
@ -164,30 +164,22 @@ async def test_autofix_review_dispatches_for_writer() -> None:
|
|||
"review": {"body": "rename to userId", "user": {"login": "alice"}},
|
||||
}
|
||||
handle = AsyncMock(return_value="dispatched")
|
||||
with (
|
||||
patch.object(webapp, "get_github_app_installation_token", AsyncMock(return_value="tok")),
|
||||
patch.object(webapp, "has_repo_write_permission", AsyncMock(return_value=True)),
|
||||
patch.object(webapp, "handle_review_feedback", handle),
|
||||
):
|
||||
with patch.object(webapp, "handle_review_feedback", handle):
|
||||
await webapp.process_github_autofix_review(payload, "pull_request_review")
|
||||
handle.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_autofix_review_skips_non_writer() -> None:
|
||||
async def test_autofix_review_delegates_permission_check_to_core() -> None:
|
||||
payload = {
|
||||
"repository": {"owner": {"login": "o"}, "name": "r"},
|
||||
"pull_request": {"number": 9, "html_url": "https://github.com/o/r/pull/9"},
|
||||
"review": {"body": "inject code", "user": {"login": "attacker"}},
|
||||
}
|
||||
handle = AsyncMock()
|
||||
with (
|
||||
patch.object(webapp, "get_github_app_installation_token", AsyncMock(return_value="tok")),
|
||||
patch.object(webapp, "has_repo_write_permission", AsyncMock(return_value=False)),
|
||||
patch.object(webapp, "handle_review_feedback", handle),
|
||||
):
|
||||
handle = AsyncMock(return_value="reviewer_no_write_permission")
|
||||
with patch.object(webapp, "handle_review_feedback", handle):
|
||||
await webapp.process_github_autofix_review(payload, "pull_request_review")
|
||||
handle.assert_not_called()
|
||||
handle.assert_awaited_once()
|
||||
|
||||
|
||||
def test_ci_events_supported() -> None:
|
||||
|
|
|
|||
|
|
@ -18,12 +18,13 @@ class _QueuedItem:
|
|||
|
||||
|
||||
class _FakeStore:
|
||||
def __init__(self, value: dict[str, Any]) -> None:
|
||||
self.value = value
|
||||
def __init__(self, items: dict[tuple[tuple[str, ...], str], dict[str, Any]]) -> None:
|
||||
self.items = items
|
||||
self.deleted: list[tuple[tuple[str, ...], str]] = []
|
||||
|
||||
async def aget(self, namespace: tuple[str, ...], key: str) -> _QueuedItem:
|
||||
return _QueuedItem(self.value)
|
||||
async def aget(self, namespace: tuple[str, ...], key: str) -> _QueuedItem | None:
|
||||
value = self.items.get((namespace, key))
|
||||
return _QueuedItem(value) if value is not None else None
|
||||
|
||||
async def adelete(self, namespace: tuple[str, ...], key: str) -> None:
|
||||
self.deleted.append((namespace, key))
|
||||
|
|
@ -33,9 +34,11 @@ class _FakeStore:
|
|||
async def test_check_message_queue_injects_dashboard_handoff_instruction() -> None:
|
||||
store = _FakeStore(
|
||||
{
|
||||
"messages": [
|
||||
{"content": {"text": "continue in web", "source": "dashboard"}},
|
||||
]
|
||||
(("queue", "thread-1"), "pending_messages"): {
|
||||
"messages": [
|
||||
{"content": {"text": "continue in web", "source": "dashboard"}},
|
||||
]
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -56,6 +59,36 @@ async def test_check_message_queue_injects_dashboard_handoff_instruction() -> No
|
|||
assert store.deleted == [(("queue", "thread-1"), "pending_messages")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_check_message_queue_injects_pending_autofix_event() -> None:
|
||||
store = _FakeStore(
|
||||
{
|
||||
(("autofix", "thread-1"), "pending_event"): {
|
||||
"reason": "review_feedback",
|
||||
"details": ["Reviewer alice commented: rename to userId"],
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"agent.middleware.check_message_queue.get_config",
|
||||
return_value={"configurable": {"thread_id": "thread-1"}},
|
||||
),
|
||||
patch("agent.middleware.check_message_queue.get_store", return_value=store),
|
||||
):
|
||||
result = await check_message_queue_before_model.abefore_model({}, MagicMock())
|
||||
|
||||
assert result is not None
|
||||
message = result["messages"][0]
|
||||
assert message["role"] == "user"
|
||||
text = message["content"][0]["text"]
|
||||
assert "PR babysitting event arrived" in text
|
||||
# The reviewer's actual comment is carried through, not dropped for a generic nudge.
|
||||
assert "rename to userId" in text
|
||||
assert (("autofix", "thread-1"), "pending_event") in store.deleted
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_blocks_skips_images_for_text_only_model() -> None:
|
||||
payload = {
|
||||
|
|
|
|||
|
|
@ -21,8 +21,11 @@ _PR = {
|
|||
def happy(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]:
|
||||
"""Patch every dependency of handle_ci_failure to a happy-path default."""
|
||||
runs_create = AsyncMock()
|
||||
store_put = AsyncMock()
|
||||
lg_client = MagicMock()
|
||||
lg_client.runs.create = runs_create
|
||||
lg_client.store.get_item = AsyncMock(return_value=None)
|
||||
lg_client.store.put_item = store_put
|
||||
threads_update = AsyncMock()
|
||||
store_client = MagicMock()
|
||||
store_client.threads.update = threads_update
|
||||
|
|
@ -31,20 +34,10 @@ def happy(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]:
|
|||
"runs_create": runs_create,
|
||||
"threads_update": threads_update,
|
||||
"status_check": AsyncMock(return_value=True),
|
||||
"queue": AsyncMock(return_value=True),
|
||||
"store_put": store_put,
|
||||
}
|
||||
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"get_autofix_settings",
|
||||
AsyncMock(
|
||||
return_value={
|
||||
"autofix_mode": "high",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"trigger_mode": "every_push",
|
||||
}
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(ci_autofix, "_user_autofix_enabled", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(ci_autofix, "is_review_repo_enabled", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(
|
||||
ci_autofix, "get_github_app_installation_token", AsyncMock(return_value="tok")
|
||||
|
|
@ -66,7 +59,6 @@ def happy(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]:
|
|||
ci_autofix, "head_commit_author_login", AsyncMock(return_value="open-swe[bot]")
|
||||
)
|
||||
monkeypatch.setattr(ci_autofix, "is_thread_active", AsyncMock(return_value=False))
|
||||
monkeypatch.setattr(ci_autofix, "queue_message_for_thread", mocks["queue"])
|
||||
monkeypatch.setattr(ci_autofix, "post_autofix_status_check", mocks["status_check"])
|
||||
monkeypatch.setattr(ci_autofix, "langgraph_client", lambda: lg_client)
|
||||
monkeypatch.setattr(ci_autofix, "get_client", lambda: store_client)
|
||||
|
|
@ -94,28 +86,21 @@ async def test_dispatch_happy_path(happy: dict[str, Any]) -> None:
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_queues_when_thread_busy(happy: dict[str, Any], monkeypatch) -> None:
|
||||
async def test_batches_when_thread_busy(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(ci_autofix, "is_thread_active", AsyncMock(return_value=True))
|
||||
result = await _run()
|
||||
assert result == "queued"
|
||||
happy["queue"].assert_awaited_once()
|
||||
assert result == "batched"
|
||||
happy["store_put"].assert_awaited()
|
||||
happy["runs_create"].assert_not_called()
|
||||
# A batched event must not burn an attempt or mark the SHA handled, so a later
|
||||
# webhook/sweep can still dispatch if the in-flight run never consumes it.
|
||||
happy["threads_update"].assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_team_disabled(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"get_autofix_settings",
|
||||
AsyncMock(
|
||||
return_value={
|
||||
"autofix_mode": "off",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"trigger_mode": "every_push",
|
||||
}
|
||||
),
|
||||
)
|
||||
assert await _run() == "autofix_disabled_team"
|
||||
async def test_skip_user_disabled(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(ci_autofix, "_user_autofix_enabled", AsyncMock(return_value=False))
|
||||
assert await _run() == "autofix_disabled_user"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -136,43 +121,6 @@ async def test_skip_no_agent_thread(happy: dict[str, Any], monkeypatch) -> None:
|
|||
assert await _run() == "no_agent_thread"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_trigger_manual(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"get_autofix_settings",
|
||||
AsyncMock(
|
||||
return_value={
|
||||
"autofix_mode": "high",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"trigger_mode": "manual",
|
||||
}
|
||||
),
|
||||
)
|
||||
assert await _run() == "trigger_manual"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_once_per_pr_after_first(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"get_autofix_settings",
|
||||
AsyncMock(
|
||||
return_value={
|
||||
"autofix_mode": "high",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"trigger_mode": "once_per_pr",
|
||||
}
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"find_agent_thread_for_pr",
|
||||
AsyncMock(return_value=("t1", {"github_login": "alice", "autofix_attempts": 1})),
|
||||
)
|
||||
assert await _run() == "once_per_pr_done"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_max_attempts(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
|
|
@ -198,7 +146,7 @@ async def test_skip_all_failing_on_base(happy: dict[str, Any], monkeypatch) -> N
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_already_handled(happy: dict[str, Any], monkeypatch) -> None:
|
||||
key = ci_autofix._dedupe_key("head1", ["lint"])
|
||||
key = ci_autofix._dedupe_key("head1")
|
||||
monkeypatch.setattr(
|
||||
ci_autofix,
|
||||
"find_agent_thread_for_pr",
|
||||
|
|
@ -227,6 +175,55 @@ async def test_ci_read_failed(happy: dict[str, Any], monkeypatch) -> None:
|
|||
assert await _run(failing_checks=None) == "ci_read_failed"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_feedback_skips_user_disabled(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(ci_autofix, "_user_autofix_enabled", AsyncMock(return_value=False))
|
||||
assert (
|
||||
await ci_autofix.handle_review_feedback(
|
||||
repo_config={"owner": "o", "name": "r"},
|
||||
pr_number=5,
|
||||
pr_url="https://github.com/o/r/pull/5",
|
||||
reviewer="alice",
|
||||
body="fix this",
|
||||
)
|
||||
== "autofix_disabled_user"
|
||||
)
|
||||
happy["runs_create"].assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_feedback_batches_when_thread_busy(happy: dict[str, Any], monkeypatch) -> None:
|
||||
monkeypatch.setattr(ci_autofix, "has_repo_write_permission", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(ci_autofix, "is_thread_active", AsyncMock(return_value=True))
|
||||
result = await ci_autofix.handle_review_feedback(
|
||||
repo_config={"owner": "o", "name": "r"},
|
||||
pr_number=5,
|
||||
pr_url="https://github.com/o/r/pull/5",
|
||||
reviewer="alice",
|
||||
body="fix this",
|
||||
)
|
||||
assert result == "batched"
|
||||
happy["runs_create"].assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_feedback_checks_write_permission_after_user_gate(
|
||||
happy: dict[str, Any], monkeypatch
|
||||
) -> None:
|
||||
permission = AsyncMock(return_value=False)
|
||||
monkeypatch.setattr(ci_autofix, "has_repo_write_permission", permission)
|
||||
result = await ci_autofix.handle_review_feedback(
|
||||
repo_config={"owner": "o", "name": "r"},
|
||||
pr_number=5,
|
||||
pr_url="https://github.com/o/r/pull/5",
|
||||
reviewer="alice",
|
||||
body="fix this",
|
||||
)
|
||||
assert result == "reviewer_no_write_permission"
|
||||
permission.assert_awaited_once()
|
||||
happy["runs_create"].assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_find_agent_thread_picks_agent_skips_reviewer(monkeypatch) -> None:
|
||||
client = MagicMock()
|
||||
|
|
|
|||
|
|
@ -110,16 +110,10 @@ export interface ProfileUpdate {
|
|||
review_draft_prs?: boolean | null
|
||||
}
|
||||
|
||||
export type TriggerMode = "every_push" | "once_per_pr" | "manual"
|
||||
export type AutofixMode = "off" | "low" | "medium" | "high"
|
||||
|
||||
export interface TeamSettings {
|
||||
trigger_mode: TriggerMode
|
||||
review_draft_prs: boolean
|
||||
pr_summaries: boolean
|
||||
review_trace_links: boolean
|
||||
autofix_mode: AutofixMode
|
||||
autofix_severity_threshold: AutofixMode
|
||||
org_guidelines?: string | null
|
||||
default_agent_model?: string | null
|
||||
default_agent_reasoning_effort?: string | null
|
||||
|
|
|
|||
|
|
@ -294,13 +294,11 @@ function CloudAgentsPage() {
|
|||
<div className="divide-y divide-border">
|
||||
<SettingsRow
|
||||
label="Automatically fix CI failures"
|
||||
description="Agent will attempt to fix failing CI checks on PRs it opens."
|
||||
comingSoon
|
||||
description="Agent will attempt to fix failing CI checks and resolve reviewer comments on PRs it opens."
|
||||
control={
|
||||
<Switch
|
||||
checked={profile.data?.auto_fix_ci ?? true}
|
||||
onCheckedChange={(v) => persist({ auto_fix_ci: v })}
|
||||
disabled
|
||||
/>
|
||||
}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -4,15 +4,8 @@ import { CaretRightIcon } from "@phosphor-icons/react";
|
|||
import { useEffect, useMemo, useState } from "react";
|
||||
import { IoLogoGithub } from "react-icons/io5";
|
||||
|
||||
import type { AutofixMode, ReposPayload, TeamSettings, TriggerMode } from "@/lib/api";
|
||||
import type { ReposPayload, TeamSettings } from "@/lib/api";
|
||||
import { AppShell, SettingsRow, SettingsSection } from "@/components/AppShell";
|
||||
import {
|
||||
Select,
|
||||
SelectContent,
|
||||
SelectItem,
|
||||
SelectTrigger,
|
||||
SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Skeleton } from "@/components/ui/skeleton";
|
||||
import { Switch } from "@/components/ui/switch";
|
||||
|
|
@ -22,38 +15,10 @@ import { useSession } from "@/lib/session";
|
|||
|
||||
export const Route = createFileRoute("/review")({ component: ReviewPage });
|
||||
|
||||
const TRIGGER_MODES: Array<{ value: TriggerMode; label: string; description: string }> = [
|
||||
{
|
||||
value: "every_push",
|
||||
label: "Every Push",
|
||||
description: "Review on every push to the PR",
|
||||
},
|
||||
{
|
||||
value: "once_per_pr",
|
||||
label: "Once Per PR",
|
||||
description: "Review once when the PR is opened, skip subsequent pushes",
|
||||
},
|
||||
{
|
||||
value: "manual",
|
||||
label: "Manual Only",
|
||||
description: "Only review when '@open-swe review' is commented",
|
||||
},
|
||||
];
|
||||
|
||||
const AUTOFIX_MODES: Array<{ value: AutofixMode; label: string }> = [
|
||||
{ value: "off", label: "Off" },
|
||||
{ value: "low", label: "Low" },
|
||||
{ value: "medium", label: "Medium" },
|
||||
{ value: "high", label: "High" },
|
||||
];
|
||||
|
||||
const DEFAULT_SETTINGS: TeamSettings = {
|
||||
trigger_mode: "every_push",
|
||||
review_draft_prs: false,
|
||||
pr_summaries: true,
|
||||
review_trace_links: true,
|
||||
autofix_mode: "off",
|
||||
autofix_severity_threshold: "medium",
|
||||
org_guidelines: null,
|
||||
default_agent_model: null,
|
||||
default_agent_reasoning_effort: null,
|
||||
|
|
@ -111,10 +76,6 @@ function ReviewPage() {
|
|||
if (canEdit) save.mutate(next);
|
||||
};
|
||||
|
||||
const triggerDescription =
|
||||
TRIGGER_MODES.find((m) => m.value === current.trigger_mode)?.description ??
|
||||
"Open SWE Review will automatically review every push to a PR";
|
||||
|
||||
const trimmedGuidelines = guidelinesDraft.trim();
|
||||
const savedGuidelines = current.org_guidelines ?? "";
|
||||
const guidelinesDirty = trimmedGuidelines !== savedGuidelines.trim();
|
||||
|
|
@ -178,29 +139,6 @@ function ReviewPage() {
|
|||
|
||||
<SettingsSection title="Configuration">
|
||||
<div className="divide-y divide-border">
|
||||
<SettingsRow
|
||||
label="Trigger Mode"
|
||||
description={triggerDescription}
|
||||
comingSoon
|
||||
control={
|
||||
<Select
|
||||
value={current.trigger_mode}
|
||||
onValueChange={(v) => persist({ trigger_mode: v as TriggerMode })}
|
||||
disabled
|
||||
>
|
||||
<SelectTrigger className="w-40">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{TRIGGER_MODES.map((m) => (
|
||||
<SelectItem key={m.value} value={m.value}>
|
||||
{m.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
}
|
||||
/>
|
||||
<SettingsRow
|
||||
label="Review Draft PRs"
|
||||
description="Org-wide default for whether Open SWE Review runs on draft PRs. Each user can override this in Profile Settings."
|
||||
|
|
@ -234,54 +172,6 @@ function ReviewPage() {
|
|||
/>
|
||||
}
|
||||
/>
|
||||
<SettingsRow
|
||||
label="Autofix Mode"
|
||||
description="When enabled, the reviewer will propose fixes. Billed at plan rates."
|
||||
comingSoon
|
||||
control={
|
||||
<Select
|
||||
value={current.autofix_mode}
|
||||
onValueChange={(v) => persist({ autofix_mode: v as AutofixMode })}
|
||||
disabled
|
||||
>
|
||||
<SelectTrigger className="w-32">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{AUTOFIX_MODES.map((m) => (
|
||||
<SelectItem key={m.value} value={m.value}>
|
||||
{m.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
}
|
||||
/>
|
||||
<SettingsRow
|
||||
label="Autofix Severity Threshold"
|
||||
description="Findings at this severity or higher are auto-fixed"
|
||||
comingSoon
|
||||
control={
|
||||
<Select
|
||||
value={current.autofix_severity_threshold}
|
||||
onValueChange={(v) =>
|
||||
persist({ autofix_severity_threshold: v as AutofixMode })
|
||||
}
|
||||
disabled
|
||||
>
|
||||
<SelectTrigger className="w-32">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{AUTOFIX_MODES.map((m) => (
|
||||
<SelectItem key={m.value} value={m.value}>
|
||||
{m.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
</SettingsSection>
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue