From 478f0c24aafa3ba44a57a832a9c64b89305511ab Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 17:53:31 -0400 Subject: [PATCH 1/8] feat(harness): scaffold harness package + AUTOFIX_HARNESS flag --- src/seer/automation/harness/__init__.py | 78 +++++++++++++++++++ src/seer/configuration.py | 17 ++++ tests/automation/harness/__init__.py | 0 .../harness/test_select_orchestrator.py | 65 ++++++++++++++++ 4 files changed, 160 insertions(+) create mode 100644 src/seer/automation/harness/__init__.py create mode 100644 tests/automation/harness/__init__.py create mode 100644 tests/automation/harness/test_select_orchestrator.py diff --git a/src/seer/automation/harness/__init__.py b/src/seer/automation/harness/__init__.py new file mode 100644 index 00000000..4397c62b --- /dev/null +++ b/src/seer/automation/harness/__init__.py @@ -0,0 +1,78 @@ +"""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. + + Raises ``ValueError`` on duplicate registration so two modules can't + silently clobber each other. + """ + if name in _REGISTRY: + raise ValueError( + f"Harness '{name}' is already registered as " + f"{_REGISTRY[name].__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"] 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_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) From 53ede97fabce52da4b4eaf2173d3e7ad7fbb3218 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 20:32:19 -0400 Subject: [PATCH 2/8] feat(harness): AiderHarness subprocess wrapper + diff adapter --- src/seer/automation/harness/aider.py | 271 ++++++++++++++++++ src/seer/automation/harness/diff_adapter.py | 36 +++ tests/automation/harness/test_aider.py | 210 ++++++++++++++ tests/automation/harness/test_diff_adapter.py | 75 +++++ 4 files changed, 592 insertions(+) create mode 100644 src/seer/automation/harness/aider.py create mode 100644 src/seer/automation/harness/diff_adapter.py create mode 100644 tests/automation/harness/test_aider.py create mode 100644 tests/automation/harness/test_diff_adapter.py diff --git a/src/seer/automation/harness/aider.py b/src/seer/automation/harness/aider.py new file mode 100644 index 00000000..ead60304 --- /dev/null +++ b/src/seer/automation/harness/aider.py @@ -0,0 +1,271 @@ +"""External coding-harness orchestrator backed by the ``aider`` CLI. + +Drop-in replacement for ``AutofixAgent``: matches the constructor signature +and the ``.run(run_config)`` -> ``str | None`` return contract, so the +existing autofix components can swap orchestrators behind a feature flag +without further code changes. Tools / memory / agent-config kwargs are +accepted for compatibility but ignored — aider has its own internal tool +loop and runs 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. + * ``--ask`` mode for diagnostic steps (root cause, solution); full + auto-commit mode for the coding step. Step is inferred from + ``run_config.memory_storage_key``. + * Captured ``git diff`` lands in ``diff_str`` on the autofix-state step + when running coding mode; ``list[FilePatch]`` parsing is deferred. + +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.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 `--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", + ): + # Stored for compatibility with the AutofixAgent contract; only + # ``context`` and ``name`` are actually used. Tools/memory are + # accepted so component code can keep its existing kwargs without + # an extra branch. + self.config = config + self.context = context + self.name = name + self._unused_tools = tools + self._unused_memory = memory + self.memory: list = [] # required by some downstream code paths + + 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 run(self, run_config: RunConfig) -> Optional[str]: + """Invoke aider with the prompt from ``run_config`` and return + its stdout (which contains the model's response text). + + Raises ``HarnessRunError`` on any subprocess / clone failure. + """ + prompt = (run_config.prompt or "").strip() + if not prompt: + raise HarnessRunError("AiderHarness.run requires run_config.prompt 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) + + if not ask_mode: + # Stash the diff for the coding step to surface in the UI. + diff_str = self._capture_diff(workdir) + if diff_str: + self._record_diff(diff_str) + + return stdout + finally: + self._cleanup(workdir) + + # ─── internal helpers ─────────────────────────────────────────────── + + 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", + "AIDER_NO_PRETTY": "1", + } + argv = [ + _AIDER_BIN, + "--no-pretty", + "--no-stream", + "--yes", + "--no-attribute-author", + "--model", + f"vertex_ai/{model_name}", + "--map-tokens", + "1024", + "--message", + prompt, + ] + if ask_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: + """Stash the captured diff onto the current autofix step so the + Sentry UI can render it. Best-effort — the context may be None + in unit tests or when invoked outside an autofix run. + """ + if self.context is None: + logger.debug("No context — skipping diff record (test/standalone mode)") + return + try: + with self.context.state.update() as state: + if state.steps: + # diff_str is the simplest payload that doesn't require + # parsing into FilePatch + Hunks; Phase 2b can upgrade. + state.steps[-1].aider_diff_str = diff_str + except Exception as exc: + logger.warning("Failed to record aider diff onto autofix state: %s", exc) + + 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/tests/automation/harness/test_aider.py b/tests/automation/harness/test_aider.py new file mode 100644 index 00000000..ecd8a91a --- /dev/null +++ b/tests/automation/harness/test_aider.py @@ -0,0 +1,210 @@ +"""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.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 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"] From 43a5bbf0480b9e643f9eb1bb35d8d95179240a9a Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 20:34:07 -0400 Subject: [PATCH 3/8] build(docker): bake aider-chat into Lightweight.Dockerfile (isolated venv) --- Lightweight.Dockerfile | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Lightweight.Dockerfile b/Lightweight.Dockerfile index de192649..090f2cc3 100644 --- a/Lightweight.Dockerfile +++ b/Lightweight.Dockerfile @@ -38,6 +38,17 @@ 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 \ + && 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 From 3a8b83405219231027b85a21dbf99c952b0cf493 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 21:13:43 -0400 Subject: [PATCH 4/8] feat(autofix): wire orchestrator selection in root_cause / solution / coding components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each component now resolves AUTOFIX_HARNESS via select_orchestrator instead of constructing AutofixAgent directly. Default behavior unchanged (builtin → AutofixAgent); aider path activates only when AUTOFIX_HARNESS=aider. --- .../automation/autofix/components/coding/component.py | 11 +++++++++-- .../autofix/components/root_cause/component.py | 11 +++++++++-- .../autofix/components/solution/component.py | 10 ++++++++-- src/seer/automation/harness/__init__.py | 11 +++++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) 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 index 4397c62b..283179b6 100644 --- a/src/seer/automation/harness/__init__.py +++ b/src/seer/automation/harness/__init__.py @@ -76,3 +76,14 @@ def select_orchestrator(harness_name: str, strict: bool = False) -> Type[Autofix 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) From d305c7341e3fc832a1f7d0389cd121ebcf9d93b6 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 21:22:01 -0400 Subject: [PATCH 5/8] test(autofix): gated dogfood integration test for AiderHarness Skipped by default; opt in with SEER_AIDER_DOGFOOD=1 to run inside the seer container after rebuilding :lightweight. Confirms aider can clone the repo and answer in --ask mode against Vertex Gemini. --- .../automation/harness/test_aider_dogfood.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tests/automation/harness/test_aider_dogfood.py 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() From 5a362488261e5a99dee186a012a8caacb0d48038 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 09:05:18 -0400 Subject: [PATCH 6/8] fix(harness): aider 0.65.0 flag set + vertex deps in venv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues found during VM live test against ghcr.io/ledoent/seer:7-merge: * `--ask` is not a flag in aider 0.65.0 — use `--chat-mode ask` instead. * Add `--no-check-update --no-show-release-notes` so aider doesn't auto-pip-install upgrades mid-run (the auto-upgrade clobbered seer's pinned tokenizers when it ran against the system Python). * Install `google-auth` and `google-cloud-aiplatform` into /opt/aider-venv so litellm's vertex_ai/* model routing can authenticate via the existing seer-vertex ADC. Without these, aider hits `ModuleNotFoundError: No module named 'google'` from litellm.llms.vertex_ai_and_google_ai_studio. Verified end-to-end on sentry-seer-1: clone + ask-mode aider call against vertex_ai/gemini-2.5-flash completes in ~9s, returns real Gemini output, costs ~$0.0008 per invocation. --- Lightweight.Dockerfile | 5 ++++- src/seer/automation/harness/aider.py | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Lightweight.Dockerfile b/Lightweight.Dockerfile index 090f2cc3..0053bc29 100644 --- a/Lightweight.Dockerfile +++ b/Lightweight.Dockerfile @@ -45,7 +45,10 @@ RUN uv pip install --system -r requirements.txt # 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 \ + && /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 diff --git a/src/seer/automation/harness/aider.py b/src/seer/automation/harness/aider.py index ead60304..d7931d1b 100644 --- a/src/seer/automation/harness/aider.py +++ b/src/seer/automation/harness/aider.py @@ -51,7 +51,7 @@ _GIT_CLONE_TIMEOUT_SECONDS = 120 # Step-name -> aider mode mapping. The diagnostic steps (root_cause, -# solution) use `--ask` so aider doesn't try to commit anything; +# 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"} @@ -182,6 +182,8 @@ def _invoke_aider(self, workdir: str, prompt: str, ask_mode: bool) -> Optional[s "--no-stream", "--yes", "--no-attribute-author", + "--no-check-update", + "--no-show-release-notes", "--model", f"vertex_ai/{model_name}", "--map-tokens", @@ -190,7 +192,8 @@ def _invoke_aider(self, workdir: str, prompt: str, ask_mode: bool) -> Optional[s prompt, ] if ask_mode: - argv.insert(-2, "--ask") + argv.insert(-2, "--chat-mode") + argv.insert(-2, "ask") else: argv.insert(-2, "--auto-commits") From 01c9625b1738e87ba53b195b2823c50e4cd8feb0 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 09:44:03 -0400 Subject: [PATCH 7/8] fix(harness): align AiderHarness with AutofixAgent contract surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot review found the standalone smoke test bypassed real component integration. The autofix components in root_cause / solution / coding touch the agent in five ways that AiderHarness was missing or breaking: * `agent.usage` summed into per-step totals via `cur.usage += agent.usage` → would AttributeError. Now zero-inits `Usage()`. * `agent.add_user_message(prompt)` called *before* `agent.run(...)` in coding + solution → no method existed. Added with same signature as `AutofixAgent.add_user_message` and a `_resolve_prompt` fallback that picks up the last user message when `run_config.prompt` is empty. * `agent.tools = []` set mid-flow in root_cause before the reasoning pass → `_unused_tools` private name made it un-settable. Now a public `self.tools` attribute. * `agent.memory` fed to `llm_client.generate_structured` as the formatter LLM's context → empty memory means the formatter has nothing to extract from. Now synthesizes an assistant `Message` with aider's stdout after each run. * Constructor accepts a pre-existing memory list (CodingComponent's `_prefill_initial_memory` seeds expand_document tool calls). Also drops the broken Pydantic attribute assignment in `_record_diff`: `BaseStep` has no `extra="allow"` config, so the runtime assignment would fail in Pydantic v2. Phase 2a logs the diff at INFO; Phase 2b adds proper `ChangesStep.changes` persistence via FilePatch parsing. New tests cover each contract method plus regression guards for the flag fixes that landed in 5a36248 (`--chat-mode ask`, `--no-check-update`). A new test_component_wiring.py source-checks each of the three components still routes through `select_orchestrator` and forwards `AUTOFIX_HARNESS_STRICT`. --- src/seer/automation/harness/aider.py | 109 +++++++++----- tests/automation/harness/test_aider.py | 138 ++++++++++++++++++ .../harness/test_component_wiring.py | 47 ++++++ 3 files changed, 257 insertions(+), 37 deletions(-) create mode 100644 tests/automation/harness/test_component_wiring.py diff --git a/src/seer/automation/harness/aider.py b/src/seer/automation/harness/aider.py index d7931d1b..c6c3d093 100644 --- a/src/seer/automation/harness/aider.py +++ b/src/seer/automation/harness/aider.py @@ -1,21 +1,22 @@ """External coding-harness orchestrator backed by the ``aider`` CLI. -Drop-in replacement for ``AutofixAgent``: matches the constructor signature -and the ``.run(run_config)`` -> ``str | None`` return contract, so the -existing autofix components can swap orchestrators behind a feature flag -without further code changes. Tools / memory / agent-config kwargs are -accepted for compatibility but ignored — aider has its own internal tool -loop and runs fresh per invocation. +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. - * ``--ask`` mode for diagnostic steps (root cause, solution); full - auto-commit mode for the coding step. Step is inferred from + * ``--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`` lands in ``diff_str`` on the autofix-state step - when running coding mode; ``list[FilePatch]`` parsing is deferred. + * 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 @@ -36,6 +37,7 @@ 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__) @@ -74,16 +76,21 @@ def __init__( memory: Any = None, name: str = "AiderHarness", ): - # Stored for compatibility with the AutofixAgent contract; only - # ``context`` and ``name`` are actually used. Tools/memory are - # accepted so component code can keep its existing kwargs without - # an extra branch. + # 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._unused_tools = tools - self._unused_memory = memory - self.memory: list = [] # required by some downstream code paths + 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 @@ -91,15 +98,26 @@ def should_continue(self, run_config: RunConfig) -> bool: """ 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 from ``run_config`` and return - its stdout (which contains the model's response text). + """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 = (run_config.prompt or "").strip() + prompt = self._resolve_prompt(run_config) if not prompt: - raise HarnessRunError("AiderHarness.run requires run_config.prompt to be non-empty") + 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" @@ -114,8 +132,13 @@ def run(self, run_config: RunConfig) -> Optional[str]: 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: - # Stash the diff for the coding step to surface in the UI. diff_str = self._capture_diff(workdir) if diff_str: self._record_diff(diff_str) @@ -126,6 +149,19 @@ def run(self, run_config: RunConfig) -> Optional[str]: # ─── 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 @@ -174,7 +210,6 @@ def _invoke_aider(self, workdir: str, prompt: str, ask_mode: bool) -> Optional[s # litellm picks these up for vertex_ai/... model routing. "VERTEXAI_PROJECT": os.environ.get("GOOGLE_CLOUD_PROJECT", ""), "VERTEXAI_LOCATION": "us-central1", - "AIDER_NO_PRETTY": "1", } argv = [ _AIDER_BIN, @@ -239,21 +274,21 @@ def _capture_diff(self, workdir: str) -> str: return "" def _record_diff(self, diff_str: str) -> None: - """Stash the captured diff onto the current autofix step so the - Sentry UI can render it. Best-effort — the context may be None - in unit tests or when invoked outside an autofix run. + """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. """ - if self.context is None: - logger.debug("No context — skipping diff record (test/standalone mode)") - return - try: - with self.context.state.update() as state: - if state.steps: - # diff_str is the simplest payload that doesn't require - # parsing into FilePatch + Hunks; Phase 2b can upgrade. - state.steps[-1].aider_diff_str = diff_str - except Exception as exc: - logger.warning("Failed to record aider diff onto autofix state: %s", exc) + 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). diff --git a/tests/automation/harness/test_aider.py b/tests/automation/harness/test_aider.py index ecd8a91a..81267335 100644 --- a/tests/automation/harness/test_aider.py +++ b/tests/automation/harness/test_aider.py @@ -10,6 +10,7 @@ import pytest +from seer.automation.agent.models import Message, Usage from seer.automation.harness.aider import ( _AIDER_TIMEOUT_SECONDS, _GIT_CLONE_TIMEOUT_SECONDS, @@ -208,3 +209,140 @@ def test_registry_lookup_finds_aider(): 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_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" From 11b3bb80694a0b36a959b8f4ab5e3aa3908413f7 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 09:44:22 -0400 Subject: [PATCH 8/8] refactor(harness): idempotent re-registration of the same orchestrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pytest test isolation and module reloads can re-import harness modules, which would trip the existing duplicate-registration guard and crash at import time even though the registration is a no-op (same name, same class). Now register_harness: * Returns early when name → cls is already in the registry (same cls). * Still raises ValueError when name → *different* cls, which is the real bug it was guarding against (two modules silently clobbering each other's registration). This is the only path the duplicate-error guard was protecting; the existing test_duplicate_registration_raises still holds because that test registers two distinct classes. --- src/seer/automation/harness/__init__.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/seer/automation/harness/__init__.py b/src/seer/automation/harness/__init__.py index 283179b6..ba4abf85 100644 --- a/src/seer/automation/harness/__init__.py +++ b/src/seer/automation/harness/__init__.py @@ -48,13 +48,18 @@ class HarnessNotAvailableError(RuntimeError): def register_harness(name: str, cls: Type[AutofixOrchestrator]) -> None: """Used by harness modules at import time to register themselves. - Raises ``ValueError`` on duplicate registration so two modules can't - silently clobber each other. + 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. """ - if name in _REGISTRY: + existing = _REGISTRY.get(name) + if existing is cls: + return + if existing is not None: raise ValueError( f"Harness '{name}' is already registered as " - f"{_REGISTRY[name].__name__}; remove the duplicate registration." + f"{existing.__name__}; remove the duplicate registration." ) _REGISTRY[name] = cls