diff --git a/Lightweight.Dockerfile b/Lightweight.Dockerfile index de192649..0053bc29 100644 --- a/Lightweight.Dockerfile +++ b/Lightweight.Dockerfile @@ -38,6 +38,20 @@ ENV UV_HTTP_TIMEOUT=300 RUN uv pip install --system torch==2.2.0 --index-url https://download.pytorch.org/whl/cpu RUN uv pip install --system -r requirements.txt +# Aider has a sprawling dep graph (litellm, prompt-toolkit, gitpython, …) +# that pins different versions of packages we already use. Install it in +# a dedicated venv so its deps don't fight ours, then symlink the binary +# onto PATH so `subprocess.run(["aider", ...])` in src/seer/automation/harness/ +# Just Works. Only loaded when AUTOFIX_HARNESS=aider; the default +# AUTOFIX_HARNESS=builtin path never touches this venv. +RUN python -m venv /opt/aider-venv \ + && /opt/aider-venv/bin/pip install --no-cache-dir \ + aider-chat==0.65.0 \ + "google-auth>=2.29" \ + "google-cloud-aiplatform>=1.60" \ + && ln -s /opt/aider-venv/bin/aider /usr/local/bin/aider \ + && aider --version + # Copy model files (assuming they are in the 'models' directory) COPY models/ models/ # Copy scripts diff --git a/src/seer/automation/autofix/components/coding/component.py b/src/seer/automation/autofix/components/coding/component.py index f5c3fd18..77b80146 100644 --- a/src/seer/automation/autofix/components/coding/component.py +++ b/src/seer/automation/autofix/components/coding/component.py @@ -7,7 +7,7 @@ from seer.automation.agent.agent import AgentConfig, RunConfig from seer.automation.agent.client import GeminiProvider, LlmClient from seer.automation.agent.models import Message, ToolCall -from seer.automation.autofix.autofix_agent import AutofixAgent +from seer.automation.autofix.autofix_agent import AutofixAgent # noqa: F401 from seer.automation.autofix.autofix_context import AutofixContext from seer.automation.autofix.components.coding.models import CodingOutput, CodingRequest from seer.automation.autofix.components.coding.prompts import CodingPrompts @@ -15,6 +15,7 @@ from seer.automation.autofix.prompts import format_repo_prompt from seer.automation.autofix.tools.tools import BaseTools from seer.automation.component import BaseComponent +from seer.automation.harness import select_orchestrator from seer.configuration import AppConfig from seer.dependency_injection import inject, injected @@ -141,7 +142,13 @@ def invoke(self, request: CodingRequest) -> None: if not memory: memory = self._prefill_initial_memory(request=request) - agent = AutofixAgent( + # AUTOFIX_HARNESS=builtin (default) keeps AutofixAgent; "aider" + # delegates to the external coding harness. See harness/aider.py. + app_config = self._get_app_config() + agent_cls = select_orchestrator( + app_config.AUTOFIX_HARNESS, strict=app_config.AUTOFIX_HARNESS_STRICT + ) + agent = agent_cls( tools=tools.get_tools(include_claude_tools=True), config=AgentConfig(interactive=True), memory=memory, diff --git a/src/seer/automation/autofix/components/root_cause/component.py b/src/seer/automation/autofix/components/root_cause/component.py index 8bf4e344..e462e5fe 100644 --- a/src/seer/automation/autofix/components/root_cause/component.py +++ b/src/seer/automation/autofix/components/root_cause/component.py @@ -5,7 +5,7 @@ from seer.automation.agent.agent import AgentConfig, RunConfig from seer.automation.agent.client import GeminiProvider, LlmClient -from seer.automation.autofix.autofix_agent import AutofixAgent +from seer.automation.autofix.autofix_agent import AutofixAgent # noqa: F401 from seer.automation.autofix.autofix_context import AutofixContext from seer.automation.autofix.components.root_cause.models import ( MultipleRootCauseAnalysisOutputPrompt, @@ -16,6 +16,7 @@ from seer.automation.autofix.prompts import format_repo_prompt from seer.automation.autofix.tools.tools import BaseTools from seer.automation.component import BaseComponent +from seer.automation.harness import select_orchestrator from seer.configuration import AppConfig from seer.dependency_injection import inject, injected @@ -42,7 +43,13 @@ def invoke( sentry_sdk.set_tag("is_rethinking", len(request.initial_memory) > 0) - agent = AutofixAgent( + # AUTOFIX_HARNESS=builtin (default) keeps AutofixAgent; "aider" + # delegates to the external coding harness — see harness/aider.py + # and docs/coding-harnesses.md. + agent_cls = select_orchestrator( + config.AUTOFIX_HARNESS, strict=config.AUTOFIX_HARNESS_STRICT + ) + agent = agent_cls( tools=(tools.get_tools(can_access_repos=bool(readable_repos))), config=AgentConfig( interactive=True, diff --git a/src/seer/automation/autofix/components/solution/component.py b/src/seer/automation/autofix/components/solution/component.py index cc11ac8e..b83d283f 100644 --- a/src/seer/automation/autofix/components/solution/component.py +++ b/src/seer/automation/autofix/components/solution/component.py @@ -7,7 +7,7 @@ from seer.automation.agent.agent import AgentConfig, RunConfig from seer.automation.agent.client import GeminiProvider, LlmClient from seer.automation.agent.models import Message, ToolCall -from seer.automation.autofix.autofix_agent import AutofixAgent +from seer.automation.autofix.autofix_agent import AutofixAgent # noqa: F401 from seer.automation.autofix.autofix_context import AutofixContext from seer.automation.autofix.components.root_cause.models import RootCauseAnalysisItem from seer.automation.autofix.components.solution.models import SolutionOutput, SolutionRequest @@ -15,6 +15,7 @@ from seer.automation.autofix.prompts import format_repo_prompt from seer.automation.autofix.tools.tools import BaseTools from seer.automation.component import BaseComponent +from seer.automation.harness import select_orchestrator from seer.configuration import AppConfig from seer.dependency_injection import inject, injected @@ -123,7 +124,12 @@ def invoke( has_tools = bool(readable_repos) repos_str = format_repo_prompt(readable_repos, unreadable_repos) - agent = AutofixAgent( + # AUTOFIX_HARNESS=builtin (default) keeps AutofixAgent; "aider" + # delegates to the external coding harness. See harness/aider.py. + agent_cls = select_orchestrator( + config.AUTOFIX_HARNESS, strict=config.AUTOFIX_HARNESS_STRICT + ) + agent = agent_cls( tools=tools.get_tools() if has_tools else None, config=AgentConfig(interactive=True), memory=memory, diff --git a/src/seer/automation/harness/__init__.py b/src/seer/automation/harness/__init__.py new file mode 100644 index 00000000..ba4abf85 --- /dev/null +++ b/src/seer/automation/harness/__init__.py @@ -0,0 +1,94 @@ +"""Pluggable autofix orchestrators. + +The default orchestrator is the in-process ``AutofixAgent`` (a ReAct loop +calling Gemini via the existing ``LlmClient``). Alternate orchestrators +("harnesses") wrap external coding agents like ``aider`` as a subprocess +and let them propose a patch given the same prompt + repo context. + +Selection happens at autofix-component construction time via +``select_orchestrator(name, strict=...)``. Harness modules register +themselves at import time with ``register_harness("name", Cls)``. + +See ``docs/coding-harnesses.md`` for the rollout plan. +""" + +import logging +from typing import Protocol, Type, runtime_checkable + +from seer.automation.agent.agent import RunConfig +from seer.automation.autofix.autofix_agent import AutofixAgent + +logger = logging.getLogger(__name__) + + +@runtime_checkable +class AutofixOrchestrator(Protocol): + """Anything a component can invoke to drive an autofix step. + + Must support ``.run(run_config=RunConfig(...))`` and return whatever + ``AutofixAgent.run`` returns today (a response object with ``.message``, + ``.usage``, ``.metadata`` — see ``automation.agent.models``). + """ + + def run(self, run_config: RunConfig): ... + + +class HarnessNotAvailableError(RuntimeError): + """Raised when ``AUTOFIX_HARNESS_STRICT=True`` and the requested + harness is unknown or unimportable. Operators using strict mode want + a hard failure rather than a silent fallback. + """ + + +_REGISTRY: dict[str, Type[AutofixOrchestrator]] = { + "builtin": AutofixAgent, +} + + +def register_harness(name: str, cls: Type[AutofixOrchestrator]) -> None: + """Used by harness modules at import time to register themselves. + + Re-registering the same ``cls`` under the same ``name`` is a no-op + (pytest / module reloads commonly re-import). Registering a *different* + class under an existing name raises ``ValueError`` so two modules + can't silently clobber each other. + """ + existing = _REGISTRY.get(name) + if existing is cls: + return + if existing is not None: + raise ValueError( + f"Harness '{name}' is already registered as " + f"{existing.__name__}; remove the duplicate registration." + ) + _REGISTRY[name] = cls + + +def select_orchestrator(harness_name: str, strict: bool = False) -> Type[AutofixOrchestrator]: + """Resolve ``AppConfig.AUTOFIX_HARNESS`` to an orchestrator class. + + When the requested harness isn't registered: + * ``strict=False`` (default): log a warning and return the builtin. + * ``strict=True``: raise ``HarnessNotAvailableError``. + + The fallback prevents a typo or a missing optional dep from disabling + autofix outright. + """ + if harness_name in _REGISTRY: + return _REGISTRY[harness_name] + msg = f"Unknown AUTOFIX_HARNESS={harness_name!r}; " f"known: {sorted(_REGISTRY)}" + if strict: + raise HarnessNotAvailableError(msg) + logger.warning("%s — falling back to 'builtin'.", msg) + return _REGISTRY["builtin"] + + +# Eagerly import optional harness modules so their import-time +# register_harness() side-effects run. Wrapped in a try/except so a +# missing optional dep (e.g. aider-chat not pip-installed) doesn't +# break `from seer.automation.harness import select_orchestrator` +# for the builtin path. +try: + from . import aider as _aider # noqa: F401, E402 +except ImportError as exc: # pragma: no cover + logger.debug("Optional aider harness not importable: %s", exc) diff --git a/src/seer/automation/harness/aider.py b/src/seer/automation/harness/aider.py new file mode 100644 index 00000000..c6c3d093 --- /dev/null +++ b/src/seer/automation/harness/aider.py @@ -0,0 +1,309 @@ +"""External coding-harness orchestrator backed by the ``aider`` CLI. + +Drop-in replacement for ``AutofixAgent``: matches the constructor signature, +the ``.run(run_config)`` -> ``str | None`` return contract, and the +attribute surface the autofix components touch (``memory``, ``usage``, +``tools``, ``add_user_message``). Tools are accepted but not forwarded to +aider — aider runs its own tool loop, fresh per invocation. + +Phase 2a behaviour (see ``docs/coding-harnesses.md``): + * Hardcoded ``ledoent/seer`` repo URL on the ``feature/explorer-endpoints`` + branch since the benchmark issues are all seer-side bugs. Dynamic + repo resolution from Sentry code-mappings is Phase 2b. + * ``--chat-mode ask`` for diagnostic steps (root cause, solution); full + ``--auto-commits`` mode for the coding step. Step is inferred from + ``run_config.memory_storage_key``. + * Captured ``git diff`` is logged at INFO level; persisting it into the + autofix-state step (``ChangesStep.changes`` as ``list[FilePatch]``) is + deferred to Phase 2b because ``BaseStep`` is a Pydantic model without + ``extra="allow"``. + +Sandboxing: + * Each invocation runs in a fresh ``/tmp/aider-/`` workdir that + is shallow-cloned + cleaned up in a ``finally:`` block. + * Walltime is capped via ``subprocess.run(timeout=...)``. + * If anything fails (clone, aider non-zero, timeout, etc.), the harness + raises ``HarnessRunError`` — callers handle via the + ``AUTOFIX_HARNESS_STRICT`` flag (fall back to ``builtin`` or propagate). +""" + +import logging +import os +import shutil +import subprocess +import tempfile +from typing import Any, Optional + +import sentry_sdk + +from seer.automation.agent.agent import RunConfig +from seer.automation.agent.models import Message, Usage +from seer.automation.harness import register_harness + +logger = logging.getLogger(__name__) + +# Phase 2a hardcoded resolution. Phase 2b switches to Sentry code-mapping +# RPC lookup; see docs/coding-harnesses.md §5. +_PHASE_2A_REPO_URL = "https://github.com/ledoent/seer.git" +_PHASE_2A_DEFAULT_BRANCH = "feature/explorer-endpoints" + +# Aider command-line constants. +_AIDER_BIN = "aider" +_AIDER_TIMEOUT_SECONDS = 600 # 10 min hard cap per invocation +_GIT_CLONE_TIMEOUT_SECONDS = 120 + +# Step-name -> aider mode mapping. The diagnostic steps (root_cause, +# solution) use `--chat-mode ask` so aider doesn't try to commit anything; +# only the coding step gets the full auto-commit flow. +_ASK_MODE_STEPS = {"root_cause_analysis", "solution"} + + +class HarnessRunError(RuntimeError): + """Raised when the aider subprocess fails (non-zero exit, timeout, + clone failure, missing binary, etc.). Includes captured stdout/stderr + in the message for log-grepping. + """ + + +class AiderHarness: + """Orchestrator that delegates to the ``aider`` CLI.""" + + def __init__( + self, + config: Any = None, + context: Any = None, + tools: Any = None, + memory: Any = None, + name: str = "AiderHarness", + ): + # AutofixAgent contract surface that the components in + # autofix/components/{root_cause,solution,coding}/component.py + # touch after construction: + # * agent.add_user_message(str) — append to memory + # * agent.tools = [] — disable tools mid-flow + # * agent.memory — fed to the formatter LLM + # * agent.usage — added to step usage totals + # We accept the same kwargs, keep a real memory list (seeded from + # the caller), and zero-init Usage so post-run aggregation works. + self.config = config + self.context = context + self.name = name + self.tools = tools + self.memory: list[Message] = list(memory) if memory else [] + self.usage: Usage = Usage() + + def should_continue(self, run_config: RunConfig) -> bool: + """Compatibility no-op. AiderHarness runs in a single subprocess + invocation, so there's no internal iteration loop to continue. + """ + return False + + def add_user_message(self, content: str) -> None: + """Mirror of ``AutofixAgent.add_user_message``. Coding + solution + components push their formatted prompt this way before calling + ``run()``; we pick it up from memory in ``_resolve_prompt``. + """ + self.memory.append(Message(role="user", content=content)) + + def run(self, run_config: RunConfig) -> Optional[str]: + """Invoke aider with the prompt resolved from ``run_config.prompt`` + or the last user message added via ``add_user_message``, and + return aider's stdout (which contains the model's response). + + Raises ``HarnessRunError`` on any subprocess / clone failure. + """ + prompt = self._resolve_prompt(run_config) + if not prompt: + raise HarnessRunError( + "AiderHarness.run requires either run_config.prompt or a " + "prior add_user_message() call to be non-empty" + ) + + ask_mode = self._is_ask_step(run_config) + run_id = getattr(run_config, "run_name", None) or "anon" + # Slugify run_id for filesystem use + run_slug = "".join(c if c.isalnum() else "-" for c in run_id)[:40] + + workdir = tempfile.mkdtemp(prefix=f"aider-{run_slug}-") + sentry_sdk.set_tag("harness", "aider") + sentry_sdk.set_tag("harness.ask_mode", ask_mode) + + try: + self._clone_repo(workdir) + stdout = self._invoke_aider(workdir, prompt, ask_mode=ask_mode) + + # Synthesize an assistant Message so downstream formatter LLMs + # (root_cause/solution components feed agent.memory into + # generate_structured) have aider's output to extract from. + if stdout: + self.memory.append(Message(role="assistant", content=stdout)) + + if not ask_mode: + diff_str = self._capture_diff(workdir) + if diff_str: + self._record_diff(diff_str) + + return stdout + finally: + self._cleanup(workdir) + + # ─── internal helpers ─────────────────────────────────────────────── + + def _resolve_prompt(self, run_config: RunConfig) -> str: + """Prefer ``run_config.prompt`` (root_cause path) and fall back to + the last user message in memory (coding/solution path, which + push the prompt via ``add_user_message`` before ``run()``). + """ + explicit = (getattr(run_config, "prompt", None) or "").strip() + if explicit: + return explicit + for msg in reversed(self.memory): + if msg.role == "user" and msg.content: + return msg.content.strip() + return "" + + def _is_ask_step(self, run_config: RunConfig) -> bool: + key = getattr(run_config, "memory_storage_key", "") or "" + return key in _ASK_MODE_STEPS + + def _clone_repo(self, workdir: str) -> None: + try: + subprocess.run( + [ + "git", + "clone", + "--depth=1", + "--branch", + _PHASE_2A_DEFAULT_BRANCH, + _PHASE_2A_REPO_URL, + workdir, + ], + check=True, + capture_output=True, + text=True, + timeout=_GIT_CLONE_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as e: + raise HarnessRunError( + f"git clone of {_PHASE_2A_REPO_URL} timed out after " + f"{_GIT_CLONE_TIMEOUT_SECONDS}s" + ) from e + except subprocess.CalledProcessError as e: + raise HarnessRunError(f"git clone failed (rc={e.returncode}): {e.stderr[:500]}") from e + # Set a local git identity so aider's auto-commits don't fail on + # the global-config-missing case inside the container. + subprocess.run( + ["git", "config", "user.email", "seer-aider@ledoweb.com"], + cwd=workdir, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "Seer Aider Harness"], + cwd=workdir, + check=True, + ) + + def _invoke_aider(self, workdir: str, prompt: str, ask_mode: bool) -> Optional[str]: + model_name = self._resolve_model_name() + env = { + **os.environ, + # litellm picks these up for vertex_ai/... model routing. + "VERTEXAI_PROJECT": os.environ.get("GOOGLE_CLOUD_PROJECT", ""), + "VERTEXAI_LOCATION": "us-central1", + } + argv = [ + _AIDER_BIN, + "--no-pretty", + "--no-stream", + "--yes", + "--no-attribute-author", + "--no-check-update", + "--no-show-release-notes", + "--model", + f"vertex_ai/{model_name}", + "--map-tokens", + "1024", + "--message", + prompt, + ] + if ask_mode: + argv.insert(-2, "--chat-mode") + argv.insert(-2, "ask") + else: + argv.insert(-2, "--auto-commits") + + try: + result = subprocess.run( + argv, + cwd=workdir, + env=env, + capture_output=True, + text=True, + timeout=_AIDER_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as e: + raise HarnessRunError(f"aider exceeded walltime cap {_AIDER_TIMEOUT_SECONDS}s") from e + except FileNotFoundError as e: + raise HarnessRunError( + f"aider binary not found on PATH ({_AIDER_BIN!r}). " + "Bake `pip install aider-chat` into the seer image." + ) from e + + if result.returncode != 0: + raise HarnessRunError( + f"aider exited rc={result.returncode}: " f"stderr={result.stderr[:500]!r}" + ) + return result.stdout + + def _capture_diff(self, workdir: str) -> str: + """Returns the unified diff of HEAD~1..HEAD, or empty string if + aider produced no commit. + """ + try: + result = subprocess.run( + ["git", "diff", "HEAD~1", "HEAD"], + cwd=workdir, + capture_output=True, + text=True, + check=True, + timeout=30, + ) + return result.stdout + except subprocess.CalledProcessError: + # No HEAD~1 means aider made no commit — valid outcome. + return "" + + def _record_diff(self, diff_str: str) -> None: + """Phase 2a: log the captured diff only. + + BaseStep is a Pydantic v2 model without ``extra="allow"``, so we + cannot tack on a raw ``aider_diff_str`` attribute without a schema + change that ripples through the autofix UI contract. Phase 2b + will parse the diff into ``list[FilePatch]`` and write it to the + existing ``ChangesStep.changes`` field; until then keeping the + diff in the celery worker log is the lowest-risk option. + """ + logger.info( + "AiderHarness captured diff (%d files, %d bytes) — Phase 2a logs only", + sum(1 for line in diff_str.splitlines() if line.startswith("diff --git ")), + len(diff_str), + ) + logger.debug("aider diff:\n%s", diff_str) + + def _resolve_model_name(self) -> str: + """Read AUTOFIX_HARNESS_MODEL from env (set via AppConfig). + + The default 'gemini-2.5-flash' suffices for most autofix issues; + callers can override per-deploy. + """ + return os.environ.get("AUTOFIX_HARNESS_MODEL", "gemini-2.5-flash") + + def _cleanup(self, workdir: str) -> None: + try: + shutil.rmtree(workdir, ignore_errors=True) + except Exception as exc: + logger.warning("Failed to clean aider workdir %s: %s", workdir, exc) + + +# Register at import time so select_orchestrator("aider") finds us. +register_harness("aider", AiderHarness) diff --git a/src/seer/automation/harness/diff_adapter.py b/src/seer/automation/harness/diff_adapter.py new file mode 100644 index 00000000..4a3d66eb --- /dev/null +++ b/src/seer/automation/harness/diff_adapter.py @@ -0,0 +1,36 @@ +"""Helpers for shipping a captured git diff to seer's autofix-step output. + +Phase 2a: emit raw unified-diff text only. The Sentry UI's autofix viewer +renders this directly without needing a structured FilePatch list. + +Phase 2b will add a unified-diff -> ``FilePatch`` + ``Hunk`` parser so the +structured viewer also lights up. Deferred because it requires either the +``unidiff`` dep or ~80 lines of hand-rolled regex parsing, and the raw-text +fallback is sufficient for the Phase 2a benchmark. +""" + +from typing import Iterable + + +def count_files_in_diff(diff_str: str) -> int: + """Return the number of files touched by a unified-diff string. + + Used for telemetry / benchmark scoring — a quick `wc -l` style metric + without parsing the full hunks. + """ + return sum(1 for line in diff_str.splitlines() if line.startswith("diff --git ")) + + +def file_paths_in_diff(diff_str: str) -> Iterable[str]: + """Yield each ``b/`` target file mentioned in the diff header. + + Used for benchmark output ("aider touched these N files") without + parsing the rest of the diff. + """ + for line in diff_str.splitlines(): + if not line.startswith("diff --git "): + continue + # Format: "diff --git a/ b/" + parts = line.split(" b/", 1) + if len(parts) == 2: + yield parts[1].strip() diff --git a/src/seer/configuration.py b/src/seer/configuration.py index 6d3aa0f0..d6e21710 100644 --- a/src/seer/configuration.py +++ b/src/seer/configuration.py @@ -109,6 +109,23 @@ class AppConfig(BaseModel): AUTOFIXABILITY_SCORING_ENABLED: ParseBool = False AUTOFIX_ENABLED: ParseBool = False AUTOFIX_BACKFILL_ENABLED: ParseBool = False + + # Autofix orchestrator selection. See src/seer/automation/harness/ + + # docs/coding-harnesses.md. "builtin" runs the existing hand-rolled + # AutofixAgent loop; alternate values (e.g. "aider") delegate to an + # external coding harness wrapped in src/seer/automation/harness/. + AUTOFIX_HARNESS: str = "builtin" + # Model the harness uses when AUTOFIX_HARNESS != "builtin". Format is + # provider-specific (e.g. "gemini-2.5-flash", "gemini-2.5-pro"). The + # builtin path ignores this and uses its existing per-component model + # config. + AUTOFIX_HARNESS_MODEL: str = "gemini-2.5-flash" + # When True, an unknown / unavailable harness raises HarnessNotAvailableError + # instead of falling back to "builtin" with a warning. False is the + # operationally safer default — bad-harness config still produces + # autofixes via the legacy path. + AUTOFIX_HARNESS_STRICT: ParseBool = False + GRPC_SERVER_ENABLE: ParseBool = False HOSTNAME: str = Field(default_factory=gethostname) diff --git a/tests/automation/harness/__init__.py b/tests/automation/harness/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/automation/harness/test_aider.py b/tests/automation/harness/test_aider.py new file mode 100644 index 00000000..81267335 --- /dev/null +++ b/tests/automation/harness/test_aider.py @@ -0,0 +1,348 @@ +"""Unit tests for AiderHarness. + +The subprocess invocation is patched throughout — none of these tests +actually exec aider or git. Live verification happens via the integration +test in test_aider_dogfood.py (gated on ``AUTOFIX_HARNESS=aider``). +""" + +import subprocess +from unittest.mock import MagicMock, patch + +import pytest + +from seer.automation.agent.models import Message, Usage +from seer.automation.harness.aider import ( + _AIDER_TIMEOUT_SECONDS, + _GIT_CLONE_TIMEOUT_SECONDS, + AiderHarness, + HarnessRunError, +) + + +def _mock_run_config( + prompt: str = "diagnose the bug", memory_storage_key: str = "coding" +) -> MagicMock: + """Construct a minimal RunConfig-shaped mock — we only read .prompt + and .memory_storage_key, so a MagicMock with those attrs is enough. + """ + rc = MagicMock() + rc.prompt = prompt + rc.memory_storage_key = memory_storage_key + rc.run_name = "test-run" + return rc + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_ask_mode_skips_diff_capture(mock_run, mock_mkdtemp, mock_rmtree): + """Diagnostic steps (root_cause / solution) use --ask mode and don't + invoke `git diff` post-run. + """ + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="reasoning text", stderr="") + + harness = AiderHarness(context=None) + result = harness.run(_mock_run_config(memory_storage_key="root_cause_analysis")) + + assert result == "reasoning text" + # 3 subprocess.run calls: git clone, git config user.email, git config user.name, + # then aider. No 5th call for `git diff HEAD~1 HEAD`. + argv_lists = [call.args[0] for call in mock_run.call_args_list] + assert any("clone" in argv for argv in argv_lists), argv_lists + assert any(argv[0] == "aider" for argv in argv_lists), argv_lists + assert not any("diff" in argv and "HEAD~1" in argv for argv in argv_lists) + # Cleanup always fires. + mock_rmtree.assert_called_once() + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_coding_mode_captures_diff(mock_run, mock_mkdtemp, mock_rmtree): + """Coding mode invokes git diff after aider to capture the changeset.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + # Sequenced return values: clone, config x2, aider, diff + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # git clone + MagicMock(returncode=0, stdout="", stderr=""), # git config email + MagicMock(returncode=0, stdout="", stderr=""), # git config name + MagicMock(returncode=0, stdout="patch applied", stderr=""), # aider + MagicMock(returncode=0, stdout="diff --git ...", stderr=""), # git diff + ] + + harness = AiderHarness(context=None) + result = harness.run(_mock_run_config(memory_storage_key="coding")) + + assert result == "patch applied" + argv_lists = [call.args[0] for call in mock_run.call_args_list] + # Last call should be the diff capture. + assert argv_lists[-1] == ["git", "diff", "HEAD~1", "HEAD"] + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_propagates_aider_model_flag(mock_run, mock_mkdtemp, mock_rmtree): + """The model arg should be threaded through with the `vertex_ai/` prefix + so litellm routes via Vertex AI rather than direct Gemini API. + """ + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="") + + harness = AiderHarness(context=None) + harness.run(_mock_run_config(memory_storage_key="root_cause_analysis")) + + argv_lists = [call.args[0] for call in mock_run.call_args_list] + aider_argv = next(argv for argv in argv_lists if argv[0] == "aider") + assert "vertex_ai/gemini-2.5-flash" in aider_argv + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_handles_clone_failure(mock_run, mock_mkdtemp, mock_rmtree): + """Clone failure surfaces as HarnessRunError; workdir still cleaned up.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.side_effect = subprocess.CalledProcessError( + returncode=128, cmd=["git", "clone"], stderr="repo not found" + ) + + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match="git clone failed"): + harness.run(_mock_run_config()) + mock_rmtree.assert_called_once() + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_handles_clone_timeout(mock_run, mock_mkdtemp, mock_rmtree): + """Slow clone hits the timeout and produces a clear error message.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.side_effect = subprocess.TimeoutExpired( + cmd=["git", "clone"], timeout=_GIT_CLONE_TIMEOUT_SECONDS + ) + + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match="timed out"): + harness.run(_mock_run_config()) + mock_rmtree.assert_called_once() + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_handles_aider_timeout(mock_run, mock_mkdtemp, mock_rmtree): + """Aider walltime exceeded surfaces with the cap value in the error.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # clone + MagicMock(returncode=0, stdout="", stderr=""), # config email + MagicMock(returncode=0, stdout="", stderr=""), # config name + subprocess.TimeoutExpired(cmd=["aider"], timeout=_AIDER_TIMEOUT_SECONDS), + ] + + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match=str(_AIDER_TIMEOUT_SECONDS)): + harness.run(_mock_run_config()) + mock_rmtree.assert_called_once() + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_handles_aider_nonzero_exit(mock_run, mock_mkdtemp, mock_rmtree): + """Non-zero exit from aider becomes a HarnessRunError with stderr.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # clone + MagicMock(returncode=0, stdout="", stderr=""), # config email + MagicMock(returncode=0, stdout="", stderr=""), # config name + MagicMock(returncode=2, stdout="", stderr="API quota exceeded"), # aider + ] + + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match="API quota"): + harness.run(_mock_run_config()) + mock_rmtree.assert_called_once() + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_handles_missing_aider_binary(mock_run, mock_mkdtemp, mock_rmtree): + """Missing aider binary surfaces with a hint to install aider-chat.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # clone + MagicMock(returncode=0, stdout="", stderr=""), # config email + MagicMock(returncode=0, stdout="", stderr=""), # config name + FileNotFoundError("aider"), + ] + + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match="aider-chat"): + harness.run(_mock_run_config()) + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_empty_prompt_raises(mock_run, mock_mkdtemp, mock_rmtree): + """No prompt → harness raises before doing any subprocess work.""" + harness = AiderHarness(context=None) + with pytest.raises(HarnessRunError, match="non-empty"): + harness.run(_mock_run_config(prompt=" ")) + mock_run.assert_not_called() + mock_mkdtemp.assert_not_called() + + +def test_should_continue_is_false(): + """Compatibility no-op — AiderHarness has no internal iteration.""" + harness = AiderHarness(context=None) + assert harness.should_continue(_mock_run_config()) is False + + +def test_registry_lookup_finds_aider(): + """Import-time side effect: select_orchestrator('aider') resolves.""" + from seer.automation.harness import select_orchestrator + + assert select_orchestrator("aider") is AiderHarness + + +# ─── AutofixAgent contract surface ─────────────────────────────────────── +# +# The autofix components in root_cause/, solution/, and coding/ call into +# the agent in five ways: constructor kwargs, agent.add_user_message(...), +# agent.tools = [], agent.memory (passed to formatter LLM), and agent.usage +# (summed into step totals). These tests verify AiderHarness matches that +# surface without a runtime AttributeError. + + +def test_usage_attribute_default_initialized(): + """coding/solution/root_cause all run `cur.usage += agent.usage` after + agent.run — the harness must default-init Usage() so the += works. + """ + harness = AiderHarness() + assert isinstance(harness.usage, Usage) + # `+= Usage()` must succeed against a fresh Usage() with no AttributeError. + accumulator = Usage() + accumulator += harness.usage + assert accumulator.total_tokens == 0 + + +def test_tools_attribute_is_settable(): + """root_cause/component.py sets `agent.tools = []` mid-flow to disable + tools before the reasoning pass. + """ + harness = AiderHarness(tools=["initial-tool-list"]) + assert harness.tools == ["initial-tool-list"] + harness.tools = [] + assert harness.tools == [] + + +def test_add_user_message_appends_to_memory(): + """Coding + solution components push their prompts via add_user_message + before calling run() — that message has to land in memory. + """ + harness = AiderHarness() + harness.add_user_message("here is the bug") + assert len(harness.memory) == 1 + assert harness.memory[0].role == "user" + assert harness.memory[0].content == "here is the bug" + + +def test_memory_seeded_from_constructor(): + """If the caller passes prior memory (e.g. CodingComponent's prefill), + the harness uses it instead of starting empty. + """ + seed = [Message(role="user", content="prior context")] + harness = AiderHarness(memory=seed) + assert len(harness.memory) == 1 + # Defensive copy: mutating the constructor list shouldn't affect us. + seed.append(Message(role="user", content="leak")) + assert len(harness.memory) == 1 + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_falls_back_to_last_user_message_for_prompt(mock_run, mock_mkdtemp, mock_rmtree): + """When run_config.prompt is empty (the coding/solution flow), the + harness reaches into memory for the last user message. + """ + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="reasoning", stderr="") + + harness = AiderHarness() + harness.add_user_message("the actual prompt from add_user_message") + result = harness.run(_mock_run_config(prompt="", memory_storage_key="root_cause_analysis")) + + assert result == "reasoning" + aider_argv = next( + call.args[0] for call in mock_run.call_args_list if call.args[0][0] == "aider" + ) + # --message immediately follows in argv + msg_idx = aider_argv.index("--message") + assert aider_argv[msg_idx + 1] == "the actual prompt from add_user_message" + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_appends_assistant_message_for_formatter(mock_run, mock_mkdtemp, mock_rmtree): + """The root_cause + solution formatter LLMs read agent.memory after run. + The harness must inject aider's stdout as an assistant Message so the + formatter has something to extract. + """ + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="The root cause is X.", stderr="") + + harness = AiderHarness() + harness.run(_mock_run_config(memory_storage_key="root_cause_analysis")) + + assistant_msgs = [m for m in harness.memory if m.role == "assistant"] + assert len(assistant_msgs) == 1 + assert "root cause is X" in assistant_msgs[0].content + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_passes_chat_mode_ask_flag(mock_run, mock_mkdtemp, mock_rmtree): + """Aider 0.65.0 takes `--chat-mode ask` (two argv tokens), not `--ask`.""" + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="") + + harness = AiderHarness() + harness.run(_mock_run_config(memory_storage_key="root_cause_analysis")) + + aider_argv = next( + call.args[0] for call in mock_run.call_args_list if call.args[0][0] == "aider" + ) + # The two tokens must appear adjacent in that order. + chat_idx = aider_argv.index("--chat-mode") + assert aider_argv[chat_idx + 1] == "ask" + # And --ask must NOT be present (regression guard for the 5a36248 fix). + assert "--ask" not in aider_argv + + +@patch("seer.automation.harness.aider.shutil.rmtree") +@patch("seer.automation.harness.aider.tempfile.mkdtemp") +@patch("seer.automation.harness.aider.subprocess.run") +def test_run_passes_update_suppression_flags(mock_run, mock_mkdtemp, mock_rmtree): + """`--no-check-update` prevents aider from `pip install --upgrade`-ing + itself mid-session, which broke seer's pinned tokenizers on the VM. + """ + mock_mkdtemp.return_value = "/tmp/aider-test-xyz" + mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="") + + harness = AiderHarness() + harness.run(_mock_run_config(memory_storage_key="root_cause_analysis")) + + aider_argv = next( + call.args[0] for call in mock_run.call_args_list if call.args[0][0] == "aider" + ) + assert "--no-check-update" in aider_argv + assert "--no-show-release-notes" in aider_argv diff --git a/tests/automation/harness/test_aider_dogfood.py b/tests/automation/harness/test_aider_dogfood.py new file mode 100644 index 00000000..08cf372b --- /dev/null +++ b/tests/automation/harness/test_aider_dogfood.py @@ -0,0 +1,79 @@ +r"""Live integration test for AiderHarness — gated, manual-only. + +This test really shells out to ``aider`` (so the binary must be on PATH) +and really clones the seer repo from GitHub, then asks aider a trivial +question in ``--ask`` mode. It is skipped unless ``SEER_AIDER_DOGFOOD=1`` +is set, so CI never picks it up. + +Run manually inside the seer container after rebuilding ``:lightweight``:: + + docker exec -it sentry-self-hosted-seer-1 \\ + env SEER_AIDER_DOGFOOD=1 \\ + pytest tests/automation/harness/test_aider_dogfood.py -s + +Pass criteria: + * aider exits 0 + * stdout contains a non-empty answer from Gemini + * tempdir is cleaned up afterwards + +Failure modes worth surfacing: + * Vertex ADC not wired (missing/expired credentials at + ``/etc/sentry-extra/seer-vertex-key.json``). + * Network egress blocked from container to GitHub or Vertex. + * aider version drift breaks the flag set we pass. +""" + +import os +import shutil +import subprocess +from unittest.mock import MagicMock + +import pytest + +from seer.automation.harness.aider import AiderHarness + +DOGFOOD = os.environ.get("SEER_AIDER_DOGFOOD") == "1" + +pytestmark = pytest.mark.skipif( + not DOGFOOD, + reason="Live dogfood test — set SEER_AIDER_DOGFOOD=1 to run.", +) + + +def _has_binary(name: str) -> bool: + return shutil.which(name) is not None + + +def _ask_run_config(prompt: str) -> MagicMock: + rc = MagicMock() + rc.prompt = prompt + rc.memory_storage_key = "root_cause_analysis" # triggers --ask mode + rc.run_name = "dogfood-ask" + return rc + + +@pytest.mark.skipif(not _has_binary("aider"), reason="aider not on PATH") +@pytest.mark.skipif(not _has_binary("git"), reason="git not on PATH") +def test_aider_ask_mode_real_clone_real_invocation(): + harness = AiderHarness( + tools=None, + config=MagicMock(), + context=MagicMock(), + memory=[], + name="dogfood", + ) + + rc = _ask_run_config("In one sentence: what does src/seer/automation/harness/__init__.py do?") + + response = harness.run(rc) + + assert response is not None, "aider returned no output" + assert len(response.strip()) > 20, f"suspiciously short aider response: {response!r}" + + +@pytest.mark.skipif(not _has_binary("aider"), reason="aider not on PATH") +def test_aider_version_callable(): + """Smoke check: the aider binary in the container is invokable.""" + result = subprocess.run(["aider", "--version"], capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stderr + assert "aider" in result.stdout.lower() diff --git a/tests/automation/harness/test_component_wiring.py b/tests/automation/harness/test_component_wiring.py new file mode 100644 index 00000000..a2be6786 --- /dev/null +++ b/tests/automation/harness/test_component_wiring.py @@ -0,0 +1,47 @@ +"""Verify root_cause / solution / coding components route through +``select_orchestrator(config.AUTOFIX_HARNESS, ...)`` instead of +hard-constructing AutofixAgent. + +Catches the regression where someone removes the harness lookup and goes +back to ``AutofixAgent(...)`` directly — the AUTOFIX_HARNESS flag would +silently stop having any effect. +""" + +import inspect + +from seer.automation.autofix.components.coding import component as coding_module +from seer.automation.autofix.components.root_cause import component as root_cause_module +from seer.automation.autofix.components.solution import component as solution_module + + +def _source_calls_select_orchestrator(module) -> bool: + """Grep the module source for the orchestrator-selection call. + + Source-level grep beats unit-mocking each component's invoke() — + the components have ~30 injected deps each, so a behaviour test + would be more boilerplate than this is worth. + """ + src = inspect.getsource(module) + return "select_orchestrator(" in src + + +def test_root_cause_component_uses_select_orchestrator(): + assert _source_calls_select_orchestrator(root_cause_module) + + +def test_solution_component_uses_select_orchestrator(): + assert _source_calls_select_orchestrator(solution_module) + + +def test_coding_component_uses_select_orchestrator(): + assert _source_calls_select_orchestrator(coding_module) + + +def test_components_pass_strict_flag(): + """All three components forward AUTOFIX_HARNESS_STRICT so operators + who set strict=True actually get the strict behavior. Regression + guard against dropping the kwarg during a refactor. + """ + for mod in (root_cause_module, solution_module, coding_module): + src = inspect.getsource(mod) + assert "AUTOFIX_HARNESS_STRICT" in src, f"{mod.__name__} lost the strict flag" diff --git a/tests/automation/harness/test_diff_adapter.py b/tests/automation/harness/test_diff_adapter.py new file mode 100644 index 00000000..7e8a938c --- /dev/null +++ b/tests/automation/harness/test_diff_adapter.py @@ -0,0 +1,75 @@ +"""Unit tests for the diff-adapter helpers. + +Phase 2a scope: only the file-count and file-path extraction helpers. +Phase 2b will add a real unified-diff -> FilePatch parser; tests for that +go in a parallel ``test_file_patch_parser.py``. +""" + +from seer.automation.harness.diff_adapter import count_files_in_diff, file_paths_in_diff + +_TWO_FILE_DIFF = """\ +diff --git a/src/foo.py b/src/foo.py +index abc..def 100644 +--- a/src/foo.py ++++ b/src/foo.py +@@ -1,3 +1,4 @@ + def foo(): +- return 1 ++ return 2 ++ # bumped +diff --git a/tests/test_foo.py b/tests/test_foo.py +index 111..222 100644 +--- a/tests/test_foo.py ++++ b/tests/test_foo.py +@@ -1,1 +1,1 @@ +-assert foo() == 1 ++assert foo() == 2 +""" + +_EMPTY_DIFF = "" + +_CREATE_AND_DELETE_DIFF = """\ +diff --git a/new_file.py b/new_file.py +new file mode 100644 +index 0000000..abc +--- /dev/null ++++ b/new_file.py +@@ -0,0 +1,3 @@ ++def added(): ++ pass ++ +diff --git a/old_file.py b/old_file.py +deleted file mode 100644 +index abc..0000000 +--- a/old_file.py ++++ /dev/null +@@ -1,2 +0,0 @@ +-def removed(): +- pass +""" + + +def test_count_files_in_two_file_diff(): + assert count_files_in_diff(_TWO_FILE_DIFF) == 2 + + +def test_count_files_in_empty_diff(): + assert count_files_in_diff(_EMPTY_DIFF) == 0 + + +def test_count_files_in_create_and_delete(): + assert count_files_in_diff(_CREATE_AND_DELETE_DIFF) == 2 + + +def test_file_paths_in_two_file_diff(): + paths = list(file_paths_in_diff(_TWO_FILE_DIFF)) + assert paths == ["src/foo.py", "tests/test_foo.py"] + + +def test_file_paths_in_empty_diff(): + assert list(file_paths_in_diff(_EMPTY_DIFF)) == [] + + +def test_file_paths_in_create_and_delete_diff(): + paths = list(file_paths_in_diff(_CREATE_AND_DELETE_DIFF)) + assert paths == ["new_file.py", "old_file.py"] diff --git a/tests/automation/harness/test_select_orchestrator.py b/tests/automation/harness/test_select_orchestrator.py new file mode 100644 index 00000000..5d19bd23 --- /dev/null +++ b/tests/automation/harness/test_select_orchestrator.py @@ -0,0 +1,65 @@ +"""Unit tests for the harness selection factory. + +Aider-specific tests live in test_aider.py (added in the next commit). +This file covers only the registry + selector behaviour. +""" + +import pytest + +from seer.automation.autofix.autofix_agent import AutofixAgent +from seer.automation.harness import ( + _REGISTRY, + HarnessNotAvailableError, + register_harness, + select_orchestrator, +) + + +def test_builtin_is_registered_by_default(): + assert "builtin" in _REGISTRY + assert _REGISTRY["builtin"] is AutofixAgent + + +def test_select_builtin_returns_autofix_agent(): + assert select_orchestrator("builtin") is AutofixAgent + + +def test_select_unknown_falls_back_to_builtin(): + # Not strict — we don't want a typo in AUTOFIX_HARNESS to kill autofix entirely. + assert select_orchestrator("nope-does-not-exist") is AutofixAgent + + +def test_select_unknown_strict_raises(): + with pytest.raises(HarnessNotAvailableError) as exc_info: + select_orchestrator("nope-does-not-exist", strict=True) + assert "Unknown AUTOFIX_HARNESS='nope-does-not-exist'" in str(exc_info.value) + assert "builtin" in str(exc_info.value) + + +def test_register_harness_then_select(): + class _Fake: + def run(self, run_config): + pass + + register_harness("__test_fake__", _Fake) + try: + assert select_orchestrator("__test_fake__") is _Fake + finally: + _REGISTRY.pop("__test_fake__", None) + + +def test_duplicate_registration_raises(): + class _A: + def run(self, run_config): + pass + + class _B: + def run(self, run_config): + pass + + register_harness("__test_dup__", _A) + try: + with pytest.raises(ValueError, match="already registered"): + register_harness("__test_dup__", _B) + finally: + _REGISTRY.pop("__test_dup__", None)