diff --git a/AGENTS.md b/AGENTS.md index 6cb08d3c..a3ba9c55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,9 +30,12 @@ make format # ruff format + ruff check --fix | `agent` | `agent.server:get_agent` | Main coding agent (Slack/Linear/GitHub-triggered). | | `reviewer` | `agent.reviewer:get_reviewer_agent` | Read-only PR reviewer. Findings model + `publish_review`. | | `analyzer` | `agent.analyzer:get_analyzer` | Learns per-repo reviewer style from historical PRs and this reviewer's own finding outcomes. | +| `ci_monitor` | `agent.ci_monitor:get_ci_monitor` | Polling fallback for CI auto-fix: each tick sweeps open agent-authored PRs for failing checks / merge conflicts via `agent.ci_autofix.sweep_open_prs`. | 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`. + ## Architecture ### Entrypoints diff --git a/INSTALLATION.md b/INSTALLATION.md index 916d6395..73dbc4c2 100644 --- a/INSTALLATION.md +++ b/INSTALLATION.md @@ -76,7 +76,8 @@ Write this down. You'll use it in the callback URL below and again in step 4 whe - Contents: Read & write - Pull requests: Read & write - Issues: Read & write - - Checks: Read & write — reports an "Open SWE Review" check run on PRs while an auto-review runs. Without it, check-run creation fails (logged, best-effort) but reviews still work. + - Checks: Read & write — reports an "Open SWE Review" check run on PRs while an auto-review runs, and reads third-party CI conclusions for the auto-fix flow (it watches failing checks on agent-authored PRs and pushes fixes). Without it, check-run creation fails (logged, best-effort) but reviews still work, and CI auto-fix is disabled. + - Commit statuses: Read-only — only needed if you enable the `Status` event below; the CI auto-fix flow reads the legacy combined commit-status API for integrations that report via statuses instead of check runs. Without it, status-based CI is silently ignored (logged as "Failed to read combined status"). - Metadata: Read-only - **Organization permissions** (required only if you plan to set `ALLOWED_GITHUB_ORGS` — see step 5 / Security): - Members: Read-only — used to verify org membership for the dashboard-login gate via `GET /orgs/{org}/memberships/{username}`. Without this permission that call returns 403, the check fails closed, and **every** dashboard login is rejected. @@ -84,6 +85,10 @@ Write this down. You'll use it in the callback URL below and again in step 4 whe - `Issue comment` - `Pull request review` - `Pull request review comment` + - `Check run` — required for CI auto-fix (watching failing GitHub Actions checks on agent PRs) + - `Check suite` — required for CI auto-fix + - `Workflow run` — required for CI auto-fix + - `Status` — optional; covers integrations that report via the legacy commit-status API 5. Click **Create GitHub App** ### 3c. Collect credentials diff --git a/agent/ci_autofix.py b/agent/ci_autofix.py new file mode 100644 index 00000000..aa40903c --- /dev/null +++ b/agent/ci_autofix.py @@ -0,0 +1,527 @@ +"""Auto-fix CI failures and review feedback on agent-authored pull requests. + +This is the shared core for "PR babysitting": when a CI check fails (or a +reviewer leaves actionable feedback) on a PR that Open SWE opened, locate the +originating agent thread and dispatch a confidence-gated fix run on it. + +Both the GitHub webhook path (:mod:`agent.webapp`) and the polling fallback +(:mod:`agent.ci_monitor`) call into here, so all the skip-rules, dedupe, and +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. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from langgraph_sdk import get_client + +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 +from .utils.github_checks import post_autofix_status_check +from .utils.github_ci import ( + fetch_open_pr_for_branch, + fetch_pr, + head_commit_author_login, + list_failing_check_runs, + list_failing_statuses, + names_failing_on_base, +) +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__) + +# Hard cap on auto-fix follow-ups per PR so a failure the agent can't resolve +# doesn't loop forever (Cursor caps at 10). +MAX_AUTOFIX_ATTEMPTS = 10 +# Keep the dedupe list bounded on thread metadata. +_MAX_HANDLED_KEYS = 30 + + +def _dedupe_key(head_sha: str, failing_names: list[str]) -> str: + return f"{head_sha}:" + ",".join(sorted(failing_names)) + + +async def find_agent_thread_for_pr(pr_url: str) -> tuple[str, dict[str, Any]] | None: + """Return ``(thread_id, metadata)`` of the agent thread that opened ``pr_url``. + + Reviewer threads are skipped — only the coding-agent thread can push fixes. + """ + if not pr_url: + return None + client = get_client() + try: + threads = await client.threads.search(metadata={"pr_url": pr_url}, limit=10) + except Exception: # noqa: BLE001 + logger.debug("Could not search threads for PR %s", pr_url, exc_info=True) + return None + for thread in threads or []: + metadata = thread.get("metadata") if isinstance(thread, dict) else None + if not isinstance(metadata, dict): + continue + if metadata.get("kind") == REVIEWER_THREAD_KIND: + continue + if metadata.get("agent_kind") != "agent": + continue + thread_id = thread.get("thread_id") or thread.get("id") + if isinstance(thread_id, str) and thread_id: + return thread_id, metadata + return None + + +def _build_ci_fix_prompt( + *, + owner: str, + repo: str, + pr_number: int, + pr_url: str, + branch: str, + head_sha: str, + failing_checks: list[dict[str, Any]], +) -> str: + lines = [] + for check in failing_checks: + name = check.get("name", "check") + conclusion = check.get("conclusion", "failure") + details = check.get("details_url") or "" + suffix = f" — {details}" if details else "" + lines.append(f"- {name} ({conclusion}){suffix}") + failing_block = "\n".join(lines) + return ( + "An automated CI check failed on a pull request you opened. Please " + "investigate and fix it.\n\n" + f"## Repository: {owner}/{repo}\n\n" + f"## Pull Request: {pr_url} (#{pr_number})\n\n" + f"## Branch: {branch}\n\n" + f"## Head commit: {head_sha}\n\n" + f"## Failing checks:\n{failing_block}\n\n" + "Instructions:\n" + "1. Make sure you are on the PR branch, then read the failing logs " + "(e.g. `GH_TOKEN=dummy gh pr checks` and `GH_TOKEN=dummy gh run view " + " --log-failed`).\n" + "2. Confidence gating — fix autonomously ONLY when the cause is clear " + "and deterministic (lint/format, type errors, missing imports, failed " + "assertions, snapshot updates, build errors). Commit and push to the " + "existing branch; do NOT open a new PR.\n" + "3. If the failure is ambiguous, flaky, infrastructure-related, appears " + "pre-existing, or needs an architectural/design decision, do NOT guess. " + "Post a short PR comment explaining what you found and what input you " + "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." + ) + + +def _build_review_feedback_prompt( + *, + owner: str, + repo: str, + pr_number: int, + pr_url: str, + reviewer: str, + body: str, +) -> str: + return ( + "A reviewer left feedback on a pull request you opened. Please respond.\n\n" + f"## Repository: {owner}/{repo}\n\n" + f"## Pull Request: {pr_url} (#{pr_number})\n\n" + f"## Reviewer: {reviewer}\n\n" + f"## Feedback:\n{body}\n\n" + "Instructions:\n" + "1. If the requested change is unambiguous (rename, typo, missing null " + "check, small refactor, add a test), make it, commit, and push to the " + "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 " + "you changed." + ) + + +async def _thread_autofix_state(metadata: dict[str, Any]) -> tuple[int, list[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 + + +async def _record_attempt( + thread_id: str, *, attempts: int, handled: list[str], dedupe_key: str, head_sha: str +) -> None: + new_handled = [*handled, dedupe_key][-_MAX_HANDLED_KEYS:] + try: + await get_client().threads.update( + thread_id=thread_id, + metadata={ + "autofix_attempts": attempts + 1, + "autofix_handled": new_handled, + "autofix_last_head_sha": head_sha, + }, + ) + except Exception: # noqa: BLE001 + logger.debug("Failed to record auto-fix attempt for thread %s", thread_id, exc_info=True) + + +# Run sources the agent's GitHub-token resolver knows how to authenticate. +_AUTH_RESOLVABLE_SOURCES = frozenset(["github", "slack", "dashboard", "linear", "schedule"]) + + +def _run_configurable( + metadata: dict[str, Any], *, repo_config: dict[str, str], pr_number: int +) -> dict[str, Any]: + """Build the run config for a fix run by reusing the PR thread's identity. + + The agent's GitHub-token resolver only authenticates known sources, so a + bespoke ``github_ci`` source would fail in non-bot-token deployments. Reuse + the originating thread's ``source`` + login/email so auth resolves exactly + as it did for the run that opened the PR. + """ + source = metadata.get("source") + if source not in _AUTH_RESOLVABLE_SOURCES: + source = "github" + configurable: dict[str, Any] = { + "source": source, + "repo": repo_config, + "pr_number": pr_number, + } + login = metadata.get("github_login") + if isinstance(login, str) and login: + configurable["github_login"] = login + email = metadata.get("triggering_user_email") + if isinstance(email, str) and email: + configurable["user_email"] = email + return configurable + + +async def _dispatch_or_queue(thread_id: str, prompt: str, *, configurable: dict[str, Any]) -> 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" + client = langgraph_client() + await client.runs.create( + thread_id, + "agent", + input={"messages": [{"role": "user", "content": prompt}]}, + config={"configurable": configurable}, + if_not_exists="create", + ) + logger.info( + "Created auto-fix run for thread %s (source=%s)", thread_id, configurable.get("source") + ) + return "dispatched" + + +async def handle_ci_failure( + *, + repo_config: dict[str, str], + branch: str, + head_sha: str, + token: str | None = None, + source: str = "github_ci", + failing_checks: list[dict[str, Any]] | None = None, + pr: dict[str, Any] | None = None, +) -> str: + """Auto-fix failing CI on an agent-authored PR. Returns a status string.""" + owner = repo_config.get("owner", "") + repo = repo_config.get("name", "") + 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" + + if token is None: + token = await get_github_app_installation_token() + if not token: + logger.warning("No GitHub App token for CI auto-fix on %s/%s", owner, repo) + return "no_token" + + if pr is None: + if not branch: + return "no_branch" + pr = await fetch_open_pr_for_branch(owner=owner, repo=repo, branch=branch, token=token) + if not pr: + return "no_open_pr" + + pr_number = pr.get("number") + if not isinstance(pr_number, int): + return "no_pr_number" + pr_url = pr.get("html_url") or pr.get("url") or "" + base_sha = (pr.get("base") or {}).get("sha", "") + branch = branch or (pr.get("head") or {}).get("ref", "") + head_sha = head_sha or (pr.get("head") or {}).get("sha", "") + if not head_sha: + return "no_head_sha" + + if await is_pr_autofix_disabled(owner, repo, pr_number): + return "pr_disabled" + + found = await find_agent_thread_for_pr(pr_url) + if found is None: + return "no_agent_thread" + thread_id, metadata = found + + attempts, handled, github_login = await _thread_autofix_state(metadata) + + 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, + repo=repo, + head_sha=head_sha, + token=token, + title="Auto-fix limit reached", + summary=( + f"Open SWE has attempted {attempts} auto-fixes on this PR and " + "stopped to avoid a loop. Push a commit or comment to continue." + ), + details_url=dashboard_thread_url(thread_id), + ) + return "max_attempts" + + if failing_checks is None: + runs = await list_failing_check_runs(owner=owner, repo=repo, ref=head_sha, token=token) + statuses = await list_failing_statuses(owner=owner, repo=repo, ref=head_sha, token=token) + if runs is None and statuses is None: + return "ci_read_failed" + failing_checks = (runs or []) + (statuses or []) + if not failing_checks: + return "no_failing_checks" + + base_failing = await names_failing_on_base( + owner=owner, repo=repo, base_sha=base_sha, token=token + ) + actionable = [c for c in failing_checks if c.get("name") not in base_failing] + 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) + if dedupe_key in handled: + return "already_handled" + + author_login = await head_commit_author_login(owner=owner, repo=repo, sha=head_sha, token=token) + if ( + author_login is not None + and author_login not in INTERNAL_BOT_LOGINS + and (not github_login or author_login.lower() != github_login.lower()) + ): + logger.info( + "Skipping CI auto-fix on %s/%s#%s: head commit %s authored by human %s", + owner, + repo, + pr_number, + head_sha, + author_login, + ) + return "human_commit" + + prompt = _build_ci_fix_prompt( + owner=owner, + repo=repo, + pr_number=pr_number, + pr_url=pr_url, + branch=branch, + head_sha=head_sha, + failing_checks=actionable, + ) + result = await _dispatch_or_queue( + thread_id, + prompt, + configurable=_run_configurable( + metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number + ), + ) + 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 + + +async def handle_review_feedback( + *, + repo_config: dict[str, str], + pr_number: int, + pr_url: str, + reviewer: str, + body: str, + token: str | None = None, + source: str = "github_review", +) -> str: + """Auto-respond to a human review comment on an agent-authored PR.""" + owner = repo_config.get("owner", "") + repo = repo_config.get("name", "") + 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): + return "pr_disabled" + + found = await find_agent_thread_for_pr(pr_url) + if found is None: + return "no_agent_thread" + thread_id, metadata = found + + prompt = _build_review_feedback_prompt( + owner=owner, + repo=repo, + pr_number=pr_number, + pr_url=pr_url, + reviewer=reviewer, + body=body, + ) + return await _dispatch_or_queue( + thread_id, + prompt, + configurable=_run_configurable( + metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number + ), + ) + + +async def sweep_open_prs() -> dict[str, int]: + """Poll open agent-authored PRs and auto-fix failing CI / flag conflicts. + + The polling fallback for deployments without reliable CI webhooks, and the + 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} + token = await get_github_app_installation_token() + if not token: + logger.warning("CI monitor sweep: no GitHub App token") + return counts + client = get_client() + try: + threads = await client.threads.search( + metadata={"agent_kind": "agent", "pr_state": "open"}, limit=100 + ) + except Exception: # noqa: BLE001 + logger.warning("CI monitor sweep: thread search failed", exc_info=True) + return counts + + for thread in threads or []: + metadata = thread.get("metadata") if isinstance(thread, dict) else None + if not isinstance(metadata, dict): + continue + repo = metadata.get("repo") + pr_number = metadata.get("pr_number") + branch = metadata.get("branch_name") + if not isinstance(repo, dict) or not isinstance(pr_number, int): + continue + owner = repo.get("owner", "") + name = repo.get("name", "") + if not owner or not name: + continue + counts["scanned"] += 1 + pr = await fetch_pr(owner=owner, repo=name, pr_number=pr_number, token=token) + if not pr: + continue + head_sha = (pr.get("head") or {}).get("sha", "") + branch = (pr.get("head") or {}).get("ref", "") or ( + branch if isinstance(branch, str) else "" + ) + if pr.get("mergeable_state") == "dirty": + counts["conflicts"] += 1 + await _flag_merge_conflict( + owner=owner, + repo=name, + pr_number=pr_number, + pr_url=pr.get("html_url") or "", + head_sha=head_sha, + token=token, + ) + continue + result = await handle_ci_failure( + repo_config={"owner": owner, "name": name}, + branch=branch, + head_sha=head_sha, + token=token, + source="ci_monitor", + pr=pr, + ) + if result == "dispatched": + counts["dispatched"] += 1 + elif result == "queued": + counts["queued"] += 1 + logger.info("CI monitor sweep complete: %s", counts) + return counts + + +async def _flag_merge_conflict( + *, owner: str, repo: str, pr_number: int, pr_url: str, head_sha: str, token: str +) -> None: + """Ask the agent to rebase a PR that has merge conflicts with its base.""" + if await is_pr_autofix_disabled(owner, repo, pr_number): + return + found = await find_agent_thread_for_pr(pr_url) + if found is None: + return + thread_id, metadata = found + if metadata.get("autofix_conflict_head") == head_sha: + return + prompt = ( + f"The pull request you opened (#{pr_number}, {pr_url}) now has merge " + "conflicts with its base branch. Rebase or merge the base branch into " + "the PR branch, resolve the conflicts carefully, and push. If a " + "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( + thread_id, + prompt, + configurable=_run_configurable( + metadata, repo_config={"owner": owner, "name": repo}, pr_number=pr_number + ), + ) + try: + await get_client().threads.update( + thread_id=thread_id, metadata={"autofix_conflict_head": head_sha} + ) + except Exception: # noqa: BLE001 + logger.debug("Failed to record conflict head for thread %s", thread_id, exc_info=True) diff --git a/agent/ci_monitor.py b/agent/ci_monitor.py new file mode 100644 index 00000000..4fa70692 --- /dev/null +++ b/agent/ci_monitor.py @@ -0,0 +1,35 @@ +"""LangGraph entrypoint that polls open agent PRs for CI failures / conflicts. + +A fallback for deployments where CI webhooks (``check_run`` / ``workflow_run``) +aren't reliably delivered, and the only path that can react to base-branch +merge conflicts (GitHub emits no webhook for those). Register it on a cron to +sweep periodically; each tick calls :func:`agent.ci_autofix.sweep_open_prs`. +""" + +from __future__ import annotations + +import logging +from typing import Any, TypedDict + +from langgraph.graph import END, START, StateGraph +from langgraph.graph.state import RunnableConfig + +from .ci_autofix import sweep_open_prs + +logger = logging.getLogger(__name__) + + +class CIMonitorState(TypedDict, total=False): + result: dict[str, Any] + + +async def _sweep(_state: CIMonitorState, _config: RunnableConfig) -> dict[str, Any]: + return {"result": await sweep_open_prs()} + + +def get_ci_monitor(config: RunnableConfig | None = None): + builder = StateGraph(CIMonitorState) + builder.add_node("sweep", _sweep) + builder.add_edge(START, "sweep") + builder.add_edge("sweep", END) + return builder.compile().with_config(config or {}) diff --git a/agent/dashboard/autofix_state.py b/agent/dashboard/autofix_state.py new file mode 100644 index 00000000..605c7ca4 --- /dev/null +++ b/agent/dashboard/autofix_state.py @@ -0,0 +1,51 @@ +"""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`. +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 +agent thread so a disable command is honored even before any fix run exists. +""" + +from __future__ import annotations + +import logging +from datetime import UTC, datetime + +from langgraph_sdk import get_client + +logger = logging.getLogger(__name__) + +AUTOFIX_PR_STATE_NAMESPACE: list[str] = ["autofix_pr_state"] + + +def _client(): + return get_client() + + +def _key(owner: str, repo: str, pr_number: int) -> str: + return f"{owner.lower()}/{repo.lower()}#{pr_number}" + + +async def is_pr_autofix_disabled(owner: str, repo: str, pr_number: int) -> bool: + """Return whether auto-fix has been turned off for a specific PR.""" + try: + item = await _client().store.get_item( + AUTOFIX_PR_STATE_NAMESPACE, _key(owner, repo, pr_number) + ) + except Exception as e: # noqa: BLE001 + logger.debug("autofix PR state lookup failed: %s", e) + return False + if item is None: + return False + value = item.get("value") if isinstance(item, dict) else getattr(item, "value", None) + return bool(value.get("disabled")) if isinstance(value, dict) else False + + +async def set_pr_autofix_disabled(owner: str, repo: str, pr_number: int, disabled: bool) -> None: + """Persist the per-PR auto-fix opt-out flag.""" + await _client().store.put_item( + AUTOFIX_PR_STATE_NAMESPACE, + _key(owner, repo, pr_number), + {"disabled": disabled, "updated_at": datetime.now(UTC).isoformat()}, + ) diff --git a/agent/dashboard/team_settings.py b/agent/dashboard/team_settings.py index 51a204c7..267c3123 100644 --- a/agent/dashboard/team_settings.py +++ b/agent/dashboard/team_settings.py @@ -242,6 +242,31 @@ async def get_team_default_model_pair( return main, subagent +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() diff --git a/agent/utils/github_checks.py b/agent/utils/github_checks.py index 5a71ec7e..35a3dd99 100644 --- a/agent/utils/github_checks.py +++ b/agent/utils/github_checks.py @@ -20,6 +20,7 @@ import httpx logger = logging.getLogger(__name__) REVIEW_CHECK_RUN_NAME = "Open SWE Review" +AUTOFIX_CHECK_RUN_NAME = "Open SWE Auto-fix" _GITHUB_API_BASE = "https://api.github.com" @@ -116,6 +117,47 @@ async def complete_review_check_run( return True +async def post_autofix_status_check( + *, + owner: str, + repo: str, + head_sha: str, + token: str, + title: str, + summary: str, + details_url: str | None = None, +) -> bool: + """Post an informational, completed ``Open SWE Auto-fix`` check on ``head_sha``. + + Completed immediately as ``neutral`` so it's non-blocking and never leaves a + dangling in-progress check that could gate branch protection. Used as the + auto-fix status channel instead of a PR comment (PR comments can trigger + ``issue_comment`` automation like Atlantis/Terraform). + """ + payload: dict[str, object] = { + "name": AUTOFIX_CHECK_RUN_NAME, + "head_sha": head_sha, + "status": "completed", + "conclusion": "neutral", + "started_at": _utc_now_iso(), + "completed_at": _utc_now_iso(), + "output": {"title": title, "summary": summary}, + } + if details_url: + payload["details_url"] = details_url + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/check-runs" + try: + async with httpx.AsyncClient() as client: + response = await client.post( + url, headers=github_headers(token), json=payload, timeout=30 + ) + response.raise_for_status() + except httpx.HTTPError: + logger.warning("Failed to post auto-fix status check for %s/%s@%s", owner, repo, head_sha) + return False + return True + + def review_check_conclusion(surfaced_count: int) -> tuple[CheckConclusion, str, str]: """Map a publish result to (conclusion, title, summary). diff --git a/agent/utils/github_ci.py b/agent/utils/github_ci.py new file mode 100644 index 00000000..1f75f662 --- /dev/null +++ b/agent/utils/github_ci.py @@ -0,0 +1,241 @@ +"""GitHub CI read helpers for auto-fixing failing checks on agent PRs. + +These read third-party CI results (GitHub Actions check runs, the legacy +combined commit status) so the auto-fix flow can detect failures, dedupe per +commit, and decide whether a failure is pre-existing on the base branch. + +All calls are best-effort: they require the GitHub App's ``Checks: Read`` +permission, and a missing permission or transient error must never break +webhook handling. +""" + +from __future__ import annotations + +import logging +from typing import Any + +import httpx + +from .github_checks import REVIEW_CHECK_RUN_NAME, github_headers + +logger = logging.getLogger(__name__) + +_GITHUB_API_BASE = "https://api.github.com" + +# Check-run conclusions that mean "this CI step did not pass" and are worth an +# auto-fix attempt. ``cancelled`` / ``stale`` / ``skipped`` are intentionally +# excluded: they're rarely a code problem the agent can fix. +FAILING_CONCLUSIONS: frozenset[str] = frozenset(["failure", "timed_out", "action_required"]) + +# Check runs Open SWE itself produces; never treat them as fixable CI. +_OPEN_SWE_CHECK_NAMES: frozenset[str] = frozenset([REVIEW_CHECK_RUN_NAME, "Open SWE Auto-fix"]) + + +class FailingCheck(dict): + """A failing check run: ``name``, ``conclusion``, ``details_url``.""" + + +async def list_failing_check_runs( + *, owner: str, repo: str, ref: str, token: str +) -> list[dict[str, Any]] | None: + """Return failing check runs on ``ref`` (commit SHA or branch). + + Returns ``None`` when the lookup fails (e.g. missing permission) so callers + can distinguish "couldn't tell" from "nothing failing". + """ + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/commits/{ref}/check-runs" + params = {"per_page": "100", "filter": "latest"} + try: + async with httpx.AsyncClient() as client: + response = await client.get( + url, headers=github_headers(token), params=params, timeout=30 + ) + response.raise_for_status() + except httpx.HTTPError: + logger.warning( + "Failed to list check runs for %s/%s@%s (Checks: Read missing?)", owner, repo, ref + ) + return None + data = response.json() + runs = data.get("check_runs") if isinstance(data, dict) else None + if not isinstance(runs, list): + return [] + failing: list[dict[str, Any]] = [] + for run in runs: + if not isinstance(run, dict): + continue + name = run.get("name") or "" + if name in _OPEN_SWE_CHECK_NAMES: + continue + if run.get("status") != "completed": + continue + if run.get("conclusion") in FAILING_CONCLUSIONS: + failing.append( + { + "name": name, + "conclusion": run.get("conclusion"), + "details_url": run.get("details_url") or run.get("html_url") or "", + } + ) + return failing + + +async def list_failing_statuses( + *, owner: str, repo: str, ref: str, token: str +) -> list[dict[str, Any]] | None: + """Return failing legacy commit statuses on ``ref`` (the ``status`` API).""" + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/commits/{ref}/status" + try: + async with httpx.AsyncClient() as client: + response = await client.get(url, headers=github_headers(token), timeout=30) + response.raise_for_status() + except httpx.HTTPError: + logger.warning("Failed to read combined status for %s/%s@%s", owner, repo, ref) + return None + data = response.json() + statuses = data.get("statuses") if isinstance(data, dict) else None + if not isinstance(statuses, list): + return [] + failing: list[dict[str, Any]] = [] + for status in statuses: + if not isinstance(status, dict): + continue + if status.get("state") in {"failure", "error"}: + failing.append( + { + "name": status.get("context") or "", + "conclusion": status.get("state"), + "details_url": status.get("target_url") or "", + } + ) + return failing + + +def _failing_names(checks: list[dict[str, Any]] | None) -> set[str]: + return {c.get("name", "") for c in (checks or []) if c.get("name")} + + +async def names_failing_on_base(*, owner: str, repo: str, base_sha: str, token: str) -> set[str]: + """Return the set of check/status names already failing on ``base_sha``. + + Used to skip auto-fix for failures inherited from the base branch (the + failure isn't introduced by the PR), matching Cursor's skip rule. + """ + if not base_sha: + return set() + checks = await list_failing_check_runs(owner=owner, repo=repo, ref=base_sha, token=token) + statuses = await list_failing_statuses(owner=owner, repo=repo, ref=base_sha, token=token) + return _failing_names(checks) | _failing_names(statuses) + + +async def fetch_open_pr_for_branch( + *, owner: str, repo: str, branch: str, token: str +) -> dict[str, Any] | None: + """Return the first open PR whose head is ``branch`` in ``owner/repo``.""" + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/pulls" + params = {"head": f"{owner}:{branch}", "state": "open", "per_page": "1"} + try: + async with httpx.AsyncClient() as client: + response = await client.get( + url, headers=github_headers(token), params=params, timeout=30 + ) + response.raise_for_status() + except httpx.HTTPError: + logger.warning("Failed to find open PR for %s/%s head=%s", owner, repo, branch) + return None + data = response.json() + if isinstance(data, list) and data and isinstance(data[0], dict): + return data[0] + return None + + +async def fetch_pr(*, owner: str, repo: str, pr_number: int, token: str) -> dict[str, Any] | None: + """Fetch full PR metadata (includes ``mergeable_state``).""" + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/pulls/{pr_number}" + try: + async with httpx.AsyncClient() as client: + response = await client.get(url, headers=github_headers(token), timeout=30) + response.raise_for_status() + except httpx.HTTPError: + logger.warning("Failed to fetch PR %s/%s#%s", owner, repo, pr_number) + return None + data = response.json() + return data if isinstance(data, dict) else None + + +async def head_commit_author_login(*, owner: str, repo: str, sha: str, token: str) -> str | None: + """Return the GitHub login that authored commit ``sha`` (or ``None``).""" + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/commits/{sha}" + try: + async with httpx.AsyncClient() as client: + response = await client.get(url, headers=github_headers(token), timeout=30) + response.raise_for_status() + except httpx.HTTPError: + logger.debug("Failed to fetch commit %s/%s@%s for author check", owner, repo, sha) + return None + data = response.json() + author = data.get("author") if isinstance(data, dict) else None + login = author.get("login") if isinstance(author, dict) else None + return login if isinstance(login, str) and login else None + + +async def has_repo_write_permission(*, owner: str, repo: str, username: str, token: str) -> bool: + """Return whether ``username`` has write/maintain/admin on ``owner/repo``. + + Used to gate the no-mention auto-fix-on-review path so a triage/read-only + reviewer can't drive code changes. Fails closed on any error. + """ + if not username: + return False + url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/collaborators/{username}/permission" + try: + async with httpx.AsyncClient() as client: + response = await client.get(url, headers=github_headers(token), timeout=30) + response.raise_for_status() + except httpx.HTTPError: + logger.info("Could not verify %s's permission on %s/%s; denying", username, owner, repo) + return False + data = response.json() + permission = data.get("permission") if isinstance(data, dict) else None + return permission in {"admin", "maintain", "write"} + + +def branch_from_check_payload(payload: dict[str, Any], event_type: str) -> str: + """Extract the head branch name from a CI webhook payload.""" + if event_type == "check_run": + suite = (payload.get("check_run") or {}).get("check_suite") or {} + return suite.get("head_branch") or "" + if event_type == "check_suite": + return (payload.get("check_suite") or {}).get("head_branch") or "" + if event_type == "workflow_run": + return (payload.get("workflow_run") or {}).get("head_branch") or "" + if event_type == "status": + branches = payload.get("branches") + if isinstance(branches, list) and branches and isinstance(branches[0], dict): + return branches[0].get("name") or "" + return "" + + +def head_sha_from_check_payload(payload: dict[str, Any], event_type: str) -> str: + """Extract the head commit SHA from a CI webhook payload.""" + if event_type == "check_run": + return (payload.get("check_run") or {}).get("head_sha") or "" + if event_type == "check_suite": + return (payload.get("check_suite") or {}).get("head_sha") or "" + if event_type == "workflow_run": + return (payload.get("workflow_run") or {}).get("head_sha") or "" + if event_type == "status": + return payload.get("sha") or "" + return "" + + +def is_failing_ci_payload(payload: dict[str, Any], event_type: str) -> bool: + """Return whether a CI webhook payload represents a completed failure.""" + if event_type in {"check_run", "check_suite", "workflow_run"}: + node = payload.get(event_type) or {} + if node.get("status") != "completed": + return False + return node.get("conclusion") in FAILING_CONCLUSIONS + if event_type == "status": + return payload.get("state") in {"failure", "error"} + return False diff --git a/agent/webapp.py b/agent/webapp.py index 4fe23385..2b2c26dd 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -5,6 +5,7 @@ import hmac import json import logging import os +import re import uuid from collections.abc import AsyncIterator from contextlib import asynccontextmanager @@ -19,15 +20,21 @@ from langchain_core.messages.content import create_text_block from langgraph_sdk import get_client from langgraph_sdk.client import LangGraphClient +from .ci_autofix import handle_ci_failure, handle_review_feedback from .dashboard import router as dashboard_router from .dashboard.agent_overrides import ( get_profile_default_repo, resolve_login_from_email_async, ) +from .dashboard.autofix_state import set_pr_autofix_disabled from .dashboard.enabled_repos import is_review_repo_enabled from .dashboard.oauth import build_settings_url from .dashboard.profiles import get_profile, get_valid_access_token, has_access_token_record -from .dashboard.team_settings import get_team_default_repo, get_team_settings +from .dashboard.team_settings import ( + get_team_default_repo, + get_team_settings, + is_autofix_enabled, +) from .dashboard.user_mappings import ( email_for_login, login_for_email, @@ -61,6 +68,12 @@ from .utils.github_app import ( get_github_app_installation_token_with_expiry, ) 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, +) from .utils.github_comments import ( OPEN_SWE_TAGS, GitHubAuthError, @@ -1588,8 +1601,14 @@ _SUPPORTED_GH_EVENTS = frozenset( "pull_request_review_comment", "pull_request_review", "push", + "check_run", + "check_suite", + "workflow_run", + "status", ] ) +# CI events the auto-fix flow listens to (subset of _SUPPORTED_GH_EVENTS). +_GH_CI_EVENTS = frozenset(["check_run", "check_suite", "workflow_run", "status"]) _SUPPORTED_GH_ISSUE_ACTIONS = frozenset(["edited", "opened", "reopened"]) _SUPPORTED_GH_PULL_REQUEST_ACTIONS = frozenset( [ @@ -2552,6 +2571,187 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: await _store_current_reviewer_run_id(thread_id, run) +async def process_github_ci_event(payload: dict[str, Any], event_type: str) -> None: + """Auto-fix failing CI on an agent-authored PR from a CI webhook.""" + if not is_failing_ci_payload(payload, event_type): + return + repo = payload.get("repository", {}) + repo_config = { + "owner": repo.get("owner", {}).get("login", "") or repo.get("owner", {}).get("name", ""), + "name": repo.get("name", ""), + } + if not repo_config["owner"] or not repo_config["name"]: + return + branch = branch_from_check_payload(payload, event_type) + head_sha = head_sha_from_check_payload(payload, event_type) + if not head_sha: + return + result = await handle_ci_failure( + repo_config=repo_config, + branch=branch, + head_sha=head_sha, + source="github_ci", + ) + logger.info( + "CI auto-fix for %s/%s@%s (%s): %s", + repo_config["owner"], + repo_config["name"], + head_sha, + event_type, + result, + ) + + +_AUTOFIX_COMMAND_RE = re.compile(r"autofix\s+(on|off)\b", re.IGNORECASE) + + +def _parse_autofix_command(comment_body: str) -> bool | None: + """Return True (disable) / False (enable) for an ``@open-swe autofix on|off`` command. + + Returns ``None`` when the comment isn't an auto-fix command. Requires an + Open SWE mention so a passing reference to "autofix off" doesn't toggle it. + """ + if not any(tag in comment_body.lower() for tag in OPEN_SWE_TAGS): + return None + match = _AUTOFIX_COMMAND_RE.search(comment_body) + if not match: + return None + return match.group(1).lower() == "off" + + +def _pr_ref_from_comment_payload(payload: dict[str, Any], event_type: str) -> dict[str, Any] | None: + """Extract ``{owner, name, number, url}`` for the PR a comment belongs to.""" + repo = payload.get("repository", {}) + owner = repo.get("owner", {}).get("login", "") + name = repo.get("name", "") + if event_type == "issue_comment": + issue = payload.get("issue", {}) + number = issue.get("number") + pr = issue.get("pull_request") or {} + url = pr.get("html_url") or issue.get("html_url") or "" + else: + pr = payload.get("pull_request", {}) + number = pr.get("number") + url = pr.get("html_url") or "" + if not owner or not name or not isinstance(number, int): + return None + return {"owner": owner, "name": name, "number": number, "url": url} + + +async def process_github_autofix_command( + payload: dict[str, Any], event_type: str, *, disabled: bool +) -> None: + """Persist an ``@open-swe autofix on|off`` per-PR toggle and acknowledge it.""" + ref = _pr_ref_from_comment_payload(payload, event_type) + if ref is None: + return + await set_pr_autofix_disabled(ref["owner"], ref["name"], ref["number"], disabled) + logger.info( + "Auto-fix %s for %s/%s#%s via comment", + "disabled" if disabled else "enabled", + ref["owner"], + ref["name"], + ref["number"], + ) + comment = payload.get("comment") or {} + comment_id = comment.get("id") + if not isinstance(comment_id, int): + return + token = await get_github_app_installation_token() + if not token: + return + try: + await react_to_github_comment( + {"owner": ref["owner"], "name": ref["name"]}, + comment_id, + event_type=event_type, + token=token, + pull_number=ref["number"], + node_id=comment.get("node_id"), + ) + except Exception: # noqa: BLE001 + logger.debug("Failed to react to auto-fix command comment", exc_info=True) + + +# GitHub author_association values that imply at least repo-member trust. Used +# as a cheap first gate before the no-mention auto-fix-on-review path; a real +# write-permission check follows in process_github_autofix_review. +_TRUSTED_REVIEW_ASSOCIATIONS = frozenset(["OWNER", "MEMBER", "COLLABORATOR"]) + + +def _is_actionable_review_payload(payload: dict[str, Any], event_type: str) -> bool: + """Return whether a review event is trusted human feedback worth auto-responding to. + + Approvals, the agent's own bot comments, and feedback from non-trusted + authors (read/triage/outside users) are not actionable — auto-fix-on-review + dispatches a write-capable run, so only repo collaborators/members/owners + may trigger it without an explicit ``@open-swe`` mention. + """ + action = payload.get("action", "") + if event_type == "pull_request_review_comment": + if action != "created": + return False + node = payload.get("comment") or {} + elif event_type == "pull_request_review": + if action != "submitted": + return False + node = payload.get("review") or {} + if node.get("state") not in {"changes_requested", "commented"}: + return False + else: + return False + if not isinstance(node, dict): + return False + reviewer = (node.get("user") or {}).get("login", "") + if reviewer in INTERNAL_BOT_LOGINS: + return False + if node.get("author_association") not in _TRUSTED_REVIEW_ASSOCIATIONS: + return False + body = node.get("body") or "" + return bool(body.strip()) + + +async def process_github_autofix_review(payload: dict[str, Any], event_type: str) -> None: + """Auto-respond to a human review/review-comment on an agent-authored PR.""" + ref = _pr_ref_from_comment_payload(payload, event_type) + if ref is None: + return + comment = payload.get("comment") or payload.get("review", {}) + reviewer = (comment.get("user") or {}).get("login", "") if isinstance(comment, dict) else "" + 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 "", + ) + return + result = await handle_review_feedback( + repo_config={"owner": ref["owner"], "name": ref["name"]}, + pr_number=ref["number"], + pr_url=ref["url"], + reviewer=reviewer, + body=body, + source="github_review", + ) + logger.info( + "Auto-fix review feedback for %s/%s#%s: %s", + ref["owner"], + ref["name"], + ref["number"], + result, + ) + + async def _refresh_thread_github_token_after_401(thread_id: str, email: str) -> str | None: """Invalidate the cached token after a 401 and try to resolve a fresh one.""" logger.warning( @@ -3116,6 +3316,15 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> background_tasks.add_task(process_github_push_event, payload) return {"status": "accepted", "message": "Processing GitHub push for reviewer watch"} + if event_type in _GH_CI_EVENTS: + 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"} + if not _is_repo_allowed(webhook_repo_config): logger.debug( "Rejecting GitHub webhook: repo '%s/%s' not in allowlist", @@ -3159,6 +3368,23 @@ async def github_webhook(request: Request, background_tasks: BackgroundTasks) -> comment = payload.get("comment") or payload.get("review", {}) comment_body = (comment.get("body") or "") if comment else "" + + is_pr_related_comment = is_pull_request_comment or event_type in { + "pull_request_review_comment", + "pull_request_review", + } + 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"} + 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_command, payload, event_type, disabled=autofix_command + ) + return {"status": "accepted", "message": "Processing auto-fix toggle"} + if ( event_type == "pull_request_review_comment" and _review_comment_reply_parent_id(payload) is not None @@ -3172,6 +3398,15 @@ 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 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"} logger.debug( "Ignoring GitHub %s%s that does not mention @openswe or @open-swe", event_type, diff --git a/langgraph.json b/langgraph.json index 5a08e676..aad47474 100644 --- a/langgraph.json +++ b/langgraph.json @@ -6,7 +6,8 @@ "agent": "agent.server:traced_agent", "reviewer": "agent.reviewer:traced_reviewer_agent", "analyzer": "agent.analyzer:traced_analyzer", - "scheduler": "agent.scheduler:get_scheduler" + "scheduler": "agent.scheduler:get_scheduler", + "ci_monitor": "agent.ci_monitor:get_ci_monitor" }, "dependencies": [ "." diff --git a/tests/test_autofix_state.py b/tests/test_autofix_state.py new file mode 100644 index 00000000..6a5b2311 --- /dev/null +++ b/tests/test_autofix_state.py @@ -0,0 +1,70 @@ +"""Unit tests for per-PR auto-fix opt-out state and team settings accessor.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from agent.dashboard import autofix_state, team_settings + + +@pytest.mark.asyncio +async def test_set_and_check_pr_disabled() -> None: + store: dict[tuple[Any, ...], Any] = {} + client = MagicMock() + + async def put_item(ns: list[str], key: str, value: dict[str, Any]) -> None: + store[(tuple(ns), key)] = value + + async def get_item(ns: list[str], key: str) -> dict[str, Any] | None: + value = store.get((tuple(ns), key)) + return {"value": value} if value is not None else None + + client.store.put_item = AsyncMock(side_effect=put_item) + client.store.get_item = AsyncMock(side_effect=get_item) + + with patch.object(autofix_state, "get_client", return_value=client): + assert await autofix_state.is_pr_autofix_disabled("O", "R", 5) is False + await autofix_state.set_pr_autofix_disabled("O", "R", 5, True) + 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 diff --git a/tests/test_autofix_webhook.py b/tests/test_autofix_webhook.py new file mode 100644 index 00000000..6181a536 --- /dev/null +++ b/tests/test_autofix_webhook.py @@ -0,0 +1,196 @@ +"""Unit tests for the auto-fix webhook helpers in agent.webapp.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest + +from agent import webapp + + +def test_parse_autofix_command() -> None: + assert webapp._parse_autofix_command("@open-swe autofix off") is True + assert webapp._parse_autofix_command("@open-swe autofix on") is False + assert webapp._parse_autofix_command("@openswe please autofix off now") is True + # Missing the mention -> not a command. + assert webapp._parse_autofix_command("autofix off") is None + # Mention but no command keyword. + assert webapp._parse_autofix_command("@open-swe fix this") is None + + +def test_pr_ref_from_issue_comment() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "issue": { + "number": 7, + "pull_request": {"html_url": "https://github.com/o/r/pull/7"}, + }, + } + ref = webapp._pr_ref_from_comment_payload(payload, "issue_comment") + assert ref == {"owner": "o", "name": "r", "number": 7, "url": "https://github.com/o/r/pull/7"} + + +def test_pr_ref_from_review_comment() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "pull_request": {"number": 9, "html_url": "https://github.com/o/r/pull/9"}, + } + ref = webapp._pr_ref_from_comment_payload(payload, "pull_request_review_comment") + assert ref["number"] == 9 + + +def test_pr_ref_none_when_not_a_pr() -> None: + payload = {"repository": {"owner": {"login": "o"}, "name": "r"}, "issue": {"number": 3}} + # issue without pull_request still yields a ref (number present); url empty. + ref = webapp._pr_ref_from_comment_payload(payload, "issue_comment") + assert ref["url"] == "" + + +def test_is_actionable_review_payload() -> None: + assert webapp._is_actionable_review_payload( + { + "action": "submitted", + "review": { + "state": "changes_requested", + "body": "fix this", + "user": {"login": "a"}, + "author_association": "MEMBER", + }, + }, + "pull_request_review", + ) + # Approval is not actionable. + assert not webapp._is_actionable_review_payload( + { + "action": "submitted", + "review": { + "state": "approved", + "body": "lgtm", + "user": {"login": "a"}, + "author_association": "MEMBER", + }, + }, + "pull_request_review", + ) + # Bot author is not actionable. + assert not webapp._is_actionable_review_payload( + { + "action": "created", + "comment": { + "body": "x", + "user": {"login": "open-swe[bot]"}, + "author_association": "MEMBER", + }, + }, + "pull_request_review_comment", + ) + # Untrusted author (read/triage/outside) is not actionable. + assert not webapp._is_actionable_review_payload( + { + "action": "created", + "comment": { + "body": "inject malicious code", + "user": {"login": "attacker"}, + "author_association": "NONE", + }, + }, + "pull_request_review_comment", + ) + # Empty body is not actionable. + assert not webapp._is_actionable_review_payload( + { + "action": "created", + "comment": {"body": " ", "user": {"login": "a"}, "author_association": "OWNER"}, + }, + "pull_request_review_comment", + ) + + +@pytest.mark.asyncio +async def test_process_github_ci_event_dispatches() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "check_run": { + "status": "completed", + "conclusion": "failure", + "head_sha": "sha1", + "check_suite": {"head_branch": "feat"}, + }, + } + handle = AsyncMock(return_value="dispatched") + with patch.object(webapp, "handle_ci_failure", handle): + await webapp.process_github_ci_event(payload, "check_run") + handle.assert_awaited_once() + kwargs = handle.await_args.kwargs + assert kwargs["repo_config"] == {"owner": "o", "name": "r"} + assert kwargs["head_sha"] == "sha1" + assert kwargs["branch"] == "feat" + + +@pytest.mark.asyncio +async def test_process_github_ci_event_ignores_success() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "check_run": {"status": "completed", "conclusion": "success", "head_sha": "s"}, + } + handle = AsyncMock() + with patch.object(webapp, "handle_ci_failure", handle): + await webapp.process_github_ci_event(payload, "check_run") + handle.assert_not_called() + + +@pytest.mark.asyncio +async def test_process_autofix_command_sets_flag() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "issue": {"number": 7, "pull_request": {"html_url": "u"}}, + "comment": {"id": 1, "node_id": "n"}, + } + setter = AsyncMock() + with ( + patch.object(webapp, "set_pr_autofix_disabled", setter), + patch.object(webapp, "get_github_app_installation_token", AsyncMock(return_value="")), + ): + await webapp.process_github_autofix_command(payload, "issue_comment", disabled=True) + setter.assert_awaited_once_with("o", "r", 7, True) + + +@pytest.mark.asyncio +async def test_autofix_review_dispatches_for_writer() -> None: + payload = { + "repository": {"owner": {"login": "o"}, "name": "r"}, + "pull_request": {"number": 9, "html_url": "https://github.com/o/r/pull/9"}, + "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), + ): + 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: + 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), + ): + await webapp.process_github_autofix_review(payload, "pull_request_review") + handle.assert_not_called() + + +def test_ci_events_supported() -> None: + for event in ("check_run", "check_suite", "workflow_run", "status"): + assert event in webapp._SUPPORTED_GH_EVENTS + assert event in webapp._GH_CI_EVENTS diff --git a/tests/test_ci_autofix.py b/tests/test_ci_autofix.py new file mode 100644 index 00000000..a72e7970 --- /dev/null +++ b/tests/test_ci_autofix.py @@ -0,0 +1,252 @@ +"""Unit tests for the CI auto-fix orchestration core.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from agent import ci_autofix + +_PR = { + "number": 5, + "html_url": "https://github.com/o/r/pull/5", + "base": {"sha": "base"}, + "head": {"ref": "feat", "sha": "head1"}, +} + + +@pytest.fixture +def happy(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + """Patch every dependency of handle_ci_failure to a happy-path default.""" + runs_create = AsyncMock() + lg_client = MagicMock() + lg_client.runs.create = runs_create + threads_update = AsyncMock() + store_client = MagicMock() + store_client.threads.update = threads_update + + mocks: dict[str, Any] = { + "runs_create": runs_create, + "threads_update": threads_update, + "status_check": AsyncMock(return_value=True), + "queue": AsyncMock(return_value=True), + } + + 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, "is_review_repo_enabled", AsyncMock(return_value=True)) + monkeypatch.setattr( + ci_autofix, "get_github_app_installation_token", AsyncMock(return_value="tok") + ) + monkeypatch.setattr(ci_autofix, "is_pr_autofix_disabled", AsyncMock(return_value=False)) + monkeypatch.setattr( + ci_autofix, + "find_agent_thread_for_pr", + AsyncMock(return_value=("t1", {"github_login": "alice", "autofix_attempts": 0})), + ) + monkeypatch.setattr( + ci_autofix, + "list_failing_check_runs", + AsyncMock(return_value=[{"name": "lint", "conclusion": "failure", "details_url": ""}]), + ) + monkeypatch.setattr(ci_autofix, "list_failing_statuses", AsyncMock(return_value=[])) + monkeypatch.setattr(ci_autofix, "names_failing_on_base", AsyncMock(return_value=set())) + monkeypatch.setattr( + 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) + return mocks + + +async def _run(**overrides: Any) -> str: + kwargs: dict[str, Any] = { + "repo_config": {"owner": "o", "name": "r"}, + "branch": "feat", + "head_sha": "head1", + "pr": _PR, + } + kwargs.update(overrides) + return await ci_autofix.handle_ci_failure(**kwargs) + + +@pytest.mark.asyncio +async def test_dispatch_happy_path(happy: dict[str, Any]) -> None: + result = await _run() + assert result == "dispatched" + happy["runs_create"].assert_awaited_once() + happy["threads_update"].assert_awaited() + happy["status_check"].assert_awaited() + + +@pytest.mark.asyncio +async def test_queues_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() + happy["runs_create"].assert_not_called() + + +@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" + + +@pytest.mark.asyncio +async def test_skip_repo_not_enabled(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "is_review_repo_enabled", AsyncMock(return_value=False)) + assert await _run() == "repo_not_enabled" + + +@pytest.mark.asyncio +async def test_skip_pr_disabled(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "is_pr_autofix_disabled", AsyncMock(return_value=True)) + assert await _run() == "pr_disabled" + + +@pytest.mark.asyncio +async def test_skip_no_agent_thread(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "find_agent_thread_for_pr", AsyncMock(return_value=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( + ci_autofix, + "find_agent_thread_for_pr", + AsyncMock( + return_value=( + "t1", + {"github_login": "alice", "autofix_attempts": ci_autofix.MAX_AUTOFIX_ATTEMPTS}, + ) + ), + ) + assert await _run() == "max_attempts" + happy["status_check"].assert_awaited() + happy["runs_create"].assert_not_called() + + +@pytest.mark.asyncio +async def test_skip_all_failing_on_base(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "names_failing_on_base", AsyncMock(return_value={"lint"})) + assert await _run() == "all_failing_on_base" + + +@pytest.mark.asyncio +async def test_skip_already_handled(happy: dict[str, Any], monkeypatch) -> None: + key = ci_autofix._dedupe_key("head1", ["lint"]) + monkeypatch.setattr( + ci_autofix, + "find_agent_thread_for_pr", + AsyncMock(return_value=("t1", {"github_login": "alice", "autofix_handled": [key]})), + ) + assert await _run() == "already_handled" + + +@pytest.mark.asyncio +async def test_skip_human_commit(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "head_commit_author_login", AsyncMock(return_value="mallory")) + assert await _run() == "human_commit" + happy["runs_create"].assert_not_called() + + +@pytest.mark.asyncio +async def test_no_failing_checks(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "list_failing_check_runs", AsyncMock(return_value=[])) + assert await _run(failing_checks=None) == "no_failing_checks" + + +@pytest.mark.asyncio +async def test_ci_read_failed(happy: dict[str, Any], monkeypatch) -> None: + monkeypatch.setattr(ci_autofix, "list_failing_check_runs", AsyncMock(return_value=None)) + monkeypatch.setattr(ci_autofix, "list_failing_statuses", AsyncMock(return_value=None)) + assert await _run(failing_checks=None) == "ci_read_failed" + + +@pytest.mark.asyncio +async def test_find_agent_thread_picks_agent_skips_reviewer(monkeypatch) -> None: + client = MagicMock() + client.threads.search = AsyncMock( + return_value=[ + {"thread_id": "rev", "metadata": {"kind": "reviewer", "agent_kind": "agent"}}, + {"thread_id": "ag", "metadata": {"agent_kind": "agent"}}, + ] + ) + monkeypatch.setattr(ci_autofix, "get_client", lambda: client) + found = await ci_autofix.find_agent_thread_for_pr("https://github.com/o/r/pull/5") + assert found is not None + assert found[0] == "ag" + + +@pytest.mark.asyncio +async def test_find_agent_thread_none_when_only_reviewer(monkeypatch) -> None: + client = MagicMock() + client.threads.search = AsyncMock( + return_value=[{"thread_id": "rev", "metadata": {"kind": "reviewer"}}] + ) + monkeypatch.setattr(ci_autofix, "get_client", lambda: client) + assert await ci_autofix.find_agent_thread_for_pr("u") is None diff --git a/tests/test_github_checks.py b/tests/test_github_checks.py index 23bd3036..cd6dcb0d 100644 --- a/tests/test_github_checks.py +++ b/tests/test_github_checks.py @@ -111,6 +111,31 @@ async def test_complete_review_check_run_patches_completed( assert body["output"]["title"] == "Found 2 potential issues" +async def test_post_autofix_status_check_completes_neutral( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(github_checks.httpx, "AsyncClient", _FakeAsyncClient) + + ok = await github_checks.post_autofix_status_check( + owner="acme", + repo="widgets", + head_sha="abc123", + token="tok", + title="Auto-fixing 1 failing check(s)", + summary="working on it", + details_url="https://example.com/thread", + ) + + assert ok is True + assert _FakeAsyncClient.last_post is not None + assert _FakeAsyncClient.last_post["url"].endswith("/repos/acme/widgets/check-runs") + body = _FakeAsyncClient.last_post["json"] + assert body["name"] == github_checks.AUTOFIX_CHECK_RUN_NAME + assert body["status"] == "completed" + assert body["conclusion"] == "neutral" + assert body["details_url"] == "https://example.com/thread" + + def test_review_check_conclusion_mapping() -> None: conclusion, title, _ = github_checks.review_check_conclusion(0) assert conclusion == "success" diff --git a/tests/test_github_ci.py b/tests/test_github_ci.py new file mode 100644 index 00000000..53defe94 --- /dev/null +++ b/tests/test_github_ci.py @@ -0,0 +1,156 @@ +"""Unit tests for GitHub CI read helpers used by the auto-fix flow.""" + +from __future__ import annotations + +from typing import Any + +import httpx +import pytest + +from agent.utils import github_ci + + +class _FakeResponse: + def __init__(self, payload: Any = None, error: bool = False) -> None: + self._payload = payload if payload is not None else {} + self._error = error + + def raise_for_status(self) -> None: + if self._error: + raise httpx.HTTPError("boom") + + def json(self) -> Any: + return self._payload + + +class _FakeClient: + response: _FakeResponse = _FakeResponse({}) + + async def __aenter__(self) -> _FakeClient: + return self + + async def __aexit__(self, *_: object) -> None: + return None + + async def get(self, url: str, **_: Any) -> _FakeResponse: + return type(self).response + + +def _patch(monkeypatch: pytest.MonkeyPatch, payload: Any, error: bool = False) -> None: + _FakeClient.response = _FakeResponse(payload, error=error) + monkeypatch.setattr(github_ci.httpx, "AsyncClient", _FakeClient) + + +def test_branch_and_sha_from_check_run() -> None: + payload = { + "check_run": { + "head_sha": "deadbeef", + "check_suite": {"head_branch": "feat/x"}, + } + } + assert github_ci.branch_from_check_payload(payload, "check_run") == "feat/x" + assert github_ci.head_sha_from_check_payload(payload, "check_run") == "deadbeef" + + +def test_branch_and_sha_from_workflow_run() -> None: + payload = {"workflow_run": {"head_sha": "abc", "head_branch": "main"}} + assert github_ci.branch_from_check_payload(payload, "workflow_run") == "main" + assert github_ci.head_sha_from_check_payload(payload, "workflow_run") == "abc" + + +def test_sha_from_status_event() -> None: + payload = {"sha": "sha1", "branches": [{"name": "b1"}]} + assert github_ci.head_sha_from_check_payload(payload, "status") == "sha1" + assert github_ci.branch_from_check_payload(payload, "status") == "b1" + + +def test_is_failing_ci_payload() -> None: + assert github_ci.is_failing_ci_payload( + {"check_run": {"status": "completed", "conclusion": "failure"}}, "check_run" + ) + assert not github_ci.is_failing_ci_payload( + {"check_run": {"status": "completed", "conclusion": "success"}}, "check_run" + ) + assert not github_ci.is_failing_ci_payload( + {"check_run": {"status": "in_progress", "conclusion": None}}, "check_run" + ) + assert github_ci.is_failing_ci_payload({"state": "failure"}, "status") + assert not github_ci.is_failing_ci_payload({"state": "pending"}, "status") + + +@pytest.mark.asyncio +async def test_list_failing_check_runs_filters(monkeypatch: pytest.MonkeyPatch) -> None: + _patch( + monkeypatch, + { + "check_runs": [ + {"name": "lint", "status": "completed", "conclusion": "failure"}, + {"name": "test", "status": "completed", "conclusion": "success"}, + {"name": "build", "status": "in_progress", "conclusion": None}, + {"name": "Open SWE Auto-fix", "status": "completed", "conclusion": "failure"}, + ] + }, + ) + failing = await github_ci.list_failing_check_runs(owner="o", repo="r", ref="sha", token="t") + assert failing is not None + names = {c["name"] for c in failing} + assert names == {"lint"} + + +@pytest.mark.asyncio +async def test_list_failing_check_runs_returns_none_on_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _patch(monkeypatch, {}, error=True) + assert await github_ci.list_failing_check_runs(owner="o", repo="r", ref="s", token="t") is None + + +@pytest.mark.asyncio +async def test_names_failing_on_base(monkeypatch: pytest.MonkeyPatch) -> None: + # Both check-runs and statuses calls return the same fake payload here; + # only the check_runs shape is populated, statuses empty. + _patch( + monkeypatch, + { + "check_runs": [ + {"name": "flaky", "status": "completed", "conclusion": "failure"}, + ], + "statuses": [], + }, + ) + names = await github_ci.names_failing_on_base(owner="o", repo="r", base_sha="base", token="t") + assert "flaky" in names + + +@pytest.mark.asyncio +async def test_names_failing_on_base_empty_when_no_base() -> None: + assert ( + await github_ci.names_failing_on_base(owner="o", repo="r", base_sha="", token="t") == set() + ) + + +@pytest.mark.asyncio +async def test_has_repo_write_permission_true(monkeypatch: pytest.MonkeyPatch) -> None: + _patch(monkeypatch, {"permission": "write"}) + assert await github_ci.has_repo_write_permission( + owner="o", repo="r", username="alice", token="t" + ) + + +@pytest.mark.asyncio +async def test_has_repo_write_permission_false_for_read(monkeypatch: pytest.MonkeyPatch) -> None: + _patch(monkeypatch, {"permission": "read"}) + assert not await github_ci.has_repo_write_permission( + owner="o", repo="r", username="bob", token="t" + ) + + +@pytest.mark.asyncio +async def test_has_repo_write_permission_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + _patch(monkeypatch, {}, error=True) + assert not await github_ci.has_repo_write_permission( + owner="o", repo="r", username="bob", token="t" + ) + assert not await github_ci.has_repo_write_permission( + owner="o", repo="r", username="", token="t" + )