From d187cb4636a9f4febb83dceeea4361ebd84895a1 Mon Sep 17 00:00:00 2001 From: Evgeny Pavlov Date: Wed, 22 Jul 2026 17:03:54 -0700 Subject: [PATCH 1/3] Test repair agent --- agents/test-repair/Dockerfile | 45 +++ agents/test-repair/README.md | 59 ++++ agents/test-repair/compose.yml | 24 ++ agents/test-repair/hackbot.toml | 10 + .../hackbot_agents/test_repair/__init__.py | 0 .../hackbot_agents/test_repair/__main__.py | 86 +++++ .../hackbot_agents/test_repair/agent.py | 313 ++++++++++++++++++ .../hackbot_agents/test_repair/config.py | 48 +++ .../hackbot_agents/test_repair/logs.py | 103 ++++++ .../hackbot_agents/test_repair/prompts.py | 56 ++++ .../hackbot_agents/test_repair/resolve.py | 259 +++++++++++++++ agents/test-repair/pyproject.toml | 30 ++ agents/test-repair/tests/__init__.py | 0 agents/test-repair/tests/test_agent_loop.py | 149 +++++++++ agents/test-repair/tests/test_resolve.py | 117 +++++++ agents/test-repair/tests/test_scaffold.py | 67 ++++ docker-compose.yml | 9 +- services/hackbot-api/app/agents.py | 11 + services/hackbot-api/app/schemas.py | 9 + services/hackbot-api/tests/test_agents.py | 27 ++ uv.lock | 26 ++ 21 files changed, 1444 insertions(+), 4 deletions(-) create mode 100644 agents/test-repair/Dockerfile create mode 100644 agents/test-repair/README.md create mode 100644 agents/test-repair/compose.yml create mode 100644 agents/test-repair/hackbot.toml create mode 100644 agents/test-repair/hackbot_agents/test_repair/__init__.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/__main__.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/agent.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/config.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/logs.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/prompts.py create mode 100644 agents/test-repair/hackbot_agents/test_repair/resolve.py create mode 100644 agents/test-repair/pyproject.toml create mode 100644 agents/test-repair/tests/__init__.py create mode 100644 agents/test-repair/tests/test_agent_loop.py create mode 100644 agents/test-repair/tests/test_resolve.py create mode 100644 agents/test-repair/tests/test_scaffold.py diff --git a/agents/test-repair/Dockerfile b/agents/test-repair/Dockerfile new file mode 100644 index 0000000000..090e34159b --- /dev/null +++ b/agents/test-repair/Dockerfile @@ -0,0 +1,45 @@ +FROM python:3.12 AS builder + +COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ + +ENV UV_PROJECT_ENVIRONMENT=/opt/venv + +WORKDIR /app + +# Install external deps without building workspace members. +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=VERSION,target=VERSION \ + uv sync --frozen --no-dev --no-install-workspace --package hackbot-agent-test-repair + +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,target=/app,rw \ + uv sync --locked --no-dev --no-editable --package hackbot-agent-test-repair + +FROM python:3.12 AS base + +COPY --from=builder /opt/venv /opt/venv +WORKDIR /app + +ENV PYTHONUNBUFFERED=1 +ENV PYTHONDONTWRITEBYTECODE=1 +ENV PATH="/opt/venv/bin:$PATH" + +FROM base AS agent + +# hackbot.toml lives at the agent root (not inside the package), so copy it into +# the working dir; the runtime discovers it there (cwd) at startup. +COPY agents/test-repair/hackbot.toml /app/hackbot.toml + +RUN useradd --create-home --shell /bin/bash agent \ + && mkdir -p /workspace \ + && chown agent:agent /workspace + +# `mach bootstrap` installs the toolchain here at runtime; put it on PATH so the +# agent's own `./mach build` (and the build_firefox tool) find rustc/clang. +ENV PATH="/home/agent/.cargo/bin:/home/agent/.mozbuild/clang/bin:${PATH}" + +USER agent + +CMD ["python", "-m", "hackbot_agents.test_repair"] diff --git a/agents/test-repair/README.md b/agents/test-repair/README.md new file mode 100644 index 0000000000..c4b9ac133e --- /dev/null +++ b/agents/test-repair/README.md @@ -0,0 +1,59 @@ +# Test Repair Agent + +Two-stage Claude agent that finds the commit which regressed a failing Firefox CI +test and proposes a fix. Agent logic in `hackbot_agents/test_repair/`. + +The pulse listener only forwards failures that already passed its regression and +flakiness filters, so the agent assumes a genuine regression and does not +re-classify. Its only input is a Taskcluster task id. + +Run the Docker command below from the repo root, with secrets in a local `.env` +(`ANTHROPIC_API_KEY`; `BUGZILLA_API_KEY` is optional). + +## Deterministic prep + +Before Claude is invoked, `resolve.py` turns the task id into everything the +investigation needs (no log parsing): + +1. Project + hg revision from the Taskcluster task. +2. The failing test groups, via mozci. +3. The revision at which the group was last green, by walking mozci push + ancestors. +4. The git commits that landed since then (head first) from the hg pushlog + + lando; capped so an old last-green can't produce an unbounded clone. + +The head (failure) commit and a depth spanning back to last-green are set as +`SOURCE_REF` / `SOURCE_DEPTH`, so the runtime shallow-clones exactly deep enough +for the agent to `git show` every candidate. The task's full and sanitized logs +are written to files for the agent to search. + +## Input + +- `FAILURE_TASKS` - a dictionary of failed Taskcluster test tasks + `{task_name: taskcluster_task_id}`. The agent resolves the push, last-green + revision and candidate commit range from the first task id itself. + +## Output + +First stage - analysis (read-only): + +- `summary.md` - a short verdict +- `analysis.md` - detailed reasoning, with evidence from the logs and diffs +- `verdict.json` - `culprit_commit`, `culprit_bug`, `recommendation` + (`backout` / `land_fix`) and `confidence` + +Second stage - fixing (only when a culprit is identified): + +- A patch in Hackbot format + +The result reports the `culprit_commit` so the caller can attribute the +regression to a developer. + +## Test the agent + +```sh +FAILURE_TASKS='{"test-linux1804-64/opt-xpcshell-1":"XyU4b_BIRdO_IeK6z_kcQg"}' \ + docker compose up test-repair-agent --build +``` + +Artifacts are written to `~/hackbot/artifacts/`. diff --git a/agents/test-repair/compose.yml b/agents/test-repair/compose.yml new file mode 100644 index 0000000000..639ec30d1d --- /dev/null +++ b/agents/test-repair/compose.yml @@ -0,0 +1,24 @@ +services: + test-repair-agent: + build: + context: ../.. + dockerfile: agents/test-repair/Dockerfile + target: agent + # Per-run inputs are interpolated by the listener/hackbot-api as env; pydantic + # AgentInputs validates them at runtime. BUGZILLA_MCP_URL is optional (Bugzilla + # is supplementary context) and left unset here. + environment: + - RUN_ID + - FAILURE_TASKS=${FAILURE_TASKS:-} + - SOURCE_REPO=/workspace/firefox + - ANTHROPIC_API_KEY=${ANTHROPIC_API_KEY:?error} + - WANDB_API_KEY=${WANDB_API_KEY:-} + # No uploader locally: summary/logs/artifacts are written under + # /artifacts/, bind-mounted to the host's ~/hackbot/artifacts. + - ARTIFACTS_DIR=/artifacts + volumes: + - workspace:/workspace + - ${HOME}/hackbot/artifacts:/artifacts + +volumes: + workspace: diff --git a/agents/test-repair/hackbot.toml b/agents/test-repair/hackbot.toml new file mode 100644 index 0000000000..355221f344 --- /dev/null +++ b/agents/test-repair/hackbot.toml @@ -0,0 +1,10 @@ +[source] +repo_url = "https://github.com/mozilla-firefox/firefox.git" +checkout_path = "/workspace/firefox" +# The agent resolves the failure commit and candidate range from the task id at +# startup, then sets SOURCE_REF (the head/failure commit) and SOURCE_DEPTH (deep +# enough to reach the last-green commit) before the runtime prepares the tree. + +[firefox] +enabled = true +objdir = "objdir-test-repair" diff --git a/agents/test-repair/hackbot_agents/test_repair/__init__.py b/agents/test-repair/hackbot_agents/test_repair/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/agents/test-repair/hackbot_agents/test_repair/__main__.py b/agents/test-repair/hackbot_agents/test_repair/__main__.py new file mode 100644 index 0000000000..04f48d5ecb --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/__main__.py @@ -0,0 +1,86 @@ +import logging +import os +import tempfile +from pathlib import Path + +from hackbot_runtime import HackbotContext, run_async +from pydantic_settings import BaseSettings, SettingsConfigDict + +from .agent import TestRepairResult +from .logs import download_failure_logs +from .resolve import Investigation, resolve_investigation + +logger = logging.getLogger(__name__) + + +class AgentInputs(BaseSettings): + # Failing Taskcluster test tasks {task_name: task_id}. The agent resolves the + # push, last-green revision and candidate commit range from the task id. + failure_tasks: dict[str, str] + bugzilla_mcp_url: str = "" + model: str | None = None + max_turns: int | None = None + + # Compose passes unset per-run inputs as empty strings; treat those as absent. + model_config = SettingsConfigDict(extra="ignore", env_ignore_empty=True) + + +def _pin_checkout(candidate_commits: list[str]) -> None: + """Pin the shallow clone to the failure commit, deep enough for the range. + + ``SOURCE_REF`` is the head (failure) commit and ``SOURCE_DEPTH`` spans back to + the last-green commit so the agent can ``git show`` every candidate. Read by + the runtime when it prepares the source tree (HackbotContext.source_repo). + """ + os.environ.setdefault("SOURCE_REF", candidate_commits[0]) + os.environ.setdefault("SOURCE_DEPTH", str(len(candidate_commits) + 1)) + logger.info( + "Pinning checkout to %s with depth %s", + os.environ["SOURCE_REF"], + os.environ["SOURCE_DEPTH"], + ) + + +async def main(ctx: HackbotContext) -> TestRepairResult: + from .agent import run_test_repair + + inputs = AgentInputs() + if not inputs.failure_tasks: + raise ValueError("failure_tasks must contain at least one task") + + task_id = next(iter(inputs.failure_tasks.values())) + logger.info("Starting test-repair for task %s", task_id) + investigation: Investigation = resolve_investigation(task_id) + _pin_checkout(investigation.candidate_commits) + + scratch_dir = Path(tempfile.mkdtemp(prefix="test-repair-")) + scratch_in = scratch_dir / "in" + scratch_out = scratch_dir / "out" + scratch_in.mkdir(parents=True, exist_ok=True) + scratch_out.mkdir(parents=True, exist_ok=True) + + logger.info("Downloading failure logs for %d task(s)", len(inputs.failure_tasks)) + task_logs = await download_failure_logs(inputs.failure_tasks, scratch_in) + + bugzilla_mcp_server = ( + {"type": "http", "url": inputs.bugzilla_mcp_url} + if inputs.bugzilla_mcp_url + else None + ) + return await run_test_repair( + bugzilla_mcp_server=bugzilla_mcp_server, + source_repo=ctx.source_repo, + fx_ctx=ctx.firefox, + investigation=investigation, + task_logs=task_logs, + scratch_out=scratch_out, + model=inputs.model, + max_turns=inputs.max_turns, + log=ctx.log_path, + verbose=True, + publish_file=ctx.publish_file, + ) + + +if __name__ == "__main__": + run_async(main) diff --git a/agents/test-repair/hackbot_agents/test_repair/agent.py b/agents/test-repair/hackbot_agents/test_repair/agent.py new file mode 100644 index 0000000000..31ab1b62fb --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/agent.py @@ -0,0 +1,313 @@ +# -*- coding: utf-8 -*- +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Test-repair agent for Firefox CI test failures. + +Blame the commit that regressed a failing test and propose a fix. The pulse +listener only forwards failures that already passed its regression and flakiness +filters, so the agent assumes a genuine regression and does not re-classify. + +A two-stage claude-agent-sdk loop. Stage 1 (analysis, read-only) inspects the +candidate commit diffs and writes a verdict naming the culprit; Stage 2 (fix) +runs when a culprit is found and proposes a source patch, which the runtime +collects into ``changes.patch``. The :class:`TestRepairResult` is +serialized into ``summary.json``'s ``findings`` and read by the notifier. +""" + +from __future__ import annotations + +import json +import sys +from collections.abc import Callable +from pathlib import Path +from typing import Literal + +from agent_tools import firefox +from agent_tools.claude_sdk import build_sdk_server +from agent_tools.firefox import FirefoxContext +from claude_agent_sdk import ( + ClaudeAgentOptions, + ClaudeSDKClient, + McpServerConfig, + ResultMessage, +) +from hackbot_runtime import AgentError, HackbotAgentResult +from hackbot_runtime.claude import Reporter + +from .config import ( + ADDITIONAL_DIRS, + ALLOWED_TOOLS, + ANALYSIS_MODEL, + BUGZILLA_READ_TOOLS, + FIREFOX_TOOLS, + FIX_MODEL, +) +from .logs import TaskLogs +from .prompts import ( + ANALYSIS_TEMPLATE, + FIX_TEMPLATE, + LAST_GREEN_LINE, +) +from .resolve import FailingGroup, Investigation + +_CLASSIFICATIONS = ("regression", "intermittent") +_RECOMMENDATIONS = ("backout", "do_not_backout", "land_fix") + + +class TestRepairResult(HackbotAgentResult): + classification: Literal["regression", "intermittent"] + recommendation: Literal["backout", "do_not_backout", "land_fix"] + culprit_commit: str | None = None + culprit_bug: int | None = None + confidence: float = 0.0 + last_green_revision: str | None = None + intermittent_bug: int | None = None + proposed_patch: bool = False + summary: str = "" + analysis: str = "" + + +def _build_options( + *, + model: str | None, + effort: str, + cwd: Path, + scratch_dir: Path, + mcp_servers: dict[str, McpServerConfig], + allowed_tools: list[str], + max_turns: int | None, +) -> ClaudeAgentOptions: + # The agent runs inside an isolated container, so tools run without + # per-command permission prompts. + return ClaudeAgentOptions( + model=model, + cwd=str(cwd), + mcp_servers=mcp_servers, + allowed_tools=allowed_tools, + disallowed_tools=["AskUserQuestion", "Task"], + add_dirs=[*ADDITIONAL_DIRS, str(scratch_dir)], + permission_mode="bypassPermissions", + effort=effort, + max_turns=max_turns, + setting_sources=[], + ) + + +async def _run_session( + reporter: Reporter, options: ClaudeAgentOptions, prompt: str +) -> ResultMessage | None: + result_msg: ResultMessage | None = None + async with ClaudeSDKClient(options=options) as client: + await client.query(prompt) + async for msg in client.receive_response(): + reporter.message(msg) + if isinstance(msg, ResultMessage): + result_msg = msg + return result_msg + + +def _check(result_msg: ResultMessage | None, stage: str) -> None: + if result_msg is None: + raise AgentError(f"{stage} stage produced no result message") + if result_msg.is_error: + raise AgentError( + f"{stage} stage failed: {result_msg.result or result_msg.subtype}" + ) + + +def _read_doc( + scratch_out: Path, + key: str, + publish_file: Callable[[str, Path, str | None], str] | None, +) -> str: + path = scratch_out / f"{key}.md" + if not path.exists(): + return "" + if publish_file is not None: + publish_file(f"{key}.md", path, "text/markdown") + return path.read_text() + + +def _read_verdict(scratch_out: Path) -> dict: + path = scratch_out / "verdict.json" + if not path.exists(): + return {} + try: + return json.loads(path.read_text()) + except (ValueError, OSError): + return {} + + +def _coerce_classification(value) -> str: + return value if value in _CLASSIFICATIONS else "regression" + + +def _coerce_recommendation(value, classification: str) -> str: + if value in _RECOMMENDATIONS: + return value + return "backout" if classification == "regression" else "do_not_backout" + + +def _as_float(value, default: float = 0.0) -> float: + try: + return float(value) + except (TypeError, ValueError): + return default + + +def _as_int(value) -> int | None: + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _assemble_result( + scratch_out: Path, + *, + last_green_revision: str | None, + total_turns: int, + total_cost: float, + publish_file: Callable[[str, Path, str | None], str] | None, +) -> TestRepairResult: + verdict = _read_verdict(scratch_out) + classification = _coerce_classification(verdict.get("classification")) + recommendation = _coerce_recommendation( + verdict.get("recommendation"), classification + ) + culprit_commit = verdict.get("culprit_commit") + return TestRepairResult( + classification=classification, + recommendation=recommendation, + culprit_commit=culprit_commit or None, + culprit_bug=_as_int(verdict.get("culprit_bug")), + confidence=_as_float(verdict.get("confidence")), + last_green_revision=last_green_revision, + proposed_patch=bool(verdict.get("proposed_patch")), + summary=_read_doc(scratch_out, "summary", publish_file), + analysis=_read_doc(scratch_out, "analysis", publish_file), + num_turns=total_turns, + total_cost_usd=total_cost, + ) + + +def _commit_lines(candidate_commits: list[str]) -> str: + return "\n".join(f"- {c}" for c in candidate_commits) + + +def _failing_tests(groups: list[FailingGroup]) -> str: + if not groups: + return "- (failing groups could not be resolved; identify them from the logs)" + return "\n".join(f"- {g.group} (e.g. {g.test})" for g in groups) + + +async def run_test_repair( + *, + bugzilla_mcp_server: McpServerConfig | None, + source_repo: Path, + fx_ctx: FirefoxContext, + investigation: Investigation, + task_logs: dict[str, TaskLogs], + scratch_out: Path, + model: str | None = None, + max_turns: int | None = None, + verbose: bool = False, + log: Path | None = None, + publish_file: Callable[[str, Path, str | None], str] | None = None, +) -> TestRepairResult: + """Blame the commit that regressed a failing test and propose a fix.""" + candidate_commits = investigation.candidate_commits + if not candidate_commits: + raise AgentError("candidate_commits must contain at least the head commit") + failure_commit = investigation.failure_commit + print( + f"[test-repair] analyzing {investigation.hg_revision} at {failure_commit}", + file=sys.stderr, + ) + + firefox_server = build_sdk_server("firefox", fx_ctx, firefox.TOOLS) + mcp_servers: dict[str, McpServerConfig] = {"firefox": firefox_server} + allowed_tools = [*ALLOWED_TOOLS, *FIREFOX_TOOLS] + # Bugzilla is optional context (searching for a related bug); wire it only + # when a broker URL is provided. + if bugzilla_mcp_server: + mcp_servers["bugzilla"] = bugzilla_mcp_server + allowed_tools += BUGZILLA_READ_TOOLS + + failure_logs = "\n".join( + f"- {name}: sanitized failures at {tl.sanitized} (start here); " + f"full log at {tl.full}" + for name, tl in task_logs.items() + ) + last_green_line = ( + LAST_GREEN_LINE.format(last_green_revision=investigation.last_green_revision) + if investigation.last_green_revision + else "" + ) + analysis_prompt = ANALYSIS_TEMPLATE.format( + failing_tests=_failing_tests(investigation.failing_groups), + harness=investigation.harness, + failure_commit=failure_commit, + commit_lines=_commit_lines(candidate_commits), + last_green_line=last_green_line, + failure_logs=failure_logs, + scratch_out=scratch_out, + ) + + total_cost = 0.0 + total_turns = 0 + scratch_dir = scratch_out.parent + + label = ( + investigation.failing_groups[0].group + if investigation.failing_groups + else investigation.hg_revision[:12] + ) + with Reporter(verbose=verbose, log_path=log) as reporter: + reporter.header(f"{label}: analysis") + analysis_opts = _build_options( + model=model or ANALYSIS_MODEL, + effort="high", + cwd=source_repo, + scratch_dir=scratch_dir, + mcp_servers=mcp_servers, + allowed_tools=allowed_tools, + max_turns=max_turns, + ) + result_msg = await _run_session(reporter, analysis_opts, analysis_prompt) + _check(result_msg, "analysis") + total_cost += result_msg.total_cost_usd or 0.0 + total_turns += result_msg.num_turns or 0 + + # Stage 2 (fix) runs when the analysis identified a culprit commit. + verdict = _read_verdict(scratch_out) + culprit_commit = verdict.get("culprit_commit") + if culprit_commit: + reporter.header(f"{label}: fix") + fix_prompt = FIX_TEMPLATE.format( + culprit_commit=culprit_commit, + scratch_out=scratch_out, + ) + fix_opts = _build_options( + model=model or FIX_MODEL, + effort="low", + cwd=source_repo, + scratch_dir=scratch_dir, + mcp_servers=mcp_servers, + allowed_tools=allowed_tools, + max_turns=max_turns, + ) + result_msg = await _run_session(reporter, fix_opts, fix_prompt) + _check(result_msg, "fix") + total_cost += result_msg.total_cost_usd or 0.0 + total_turns += result_msg.num_turns or 0 + + return _assemble_result( + scratch_out, + last_green_revision=investigation.last_green_revision, + total_turns=total_turns, + total_cost=total_cost, + publish_file=publish_file, + ) diff --git a/agents/test-repair/hackbot_agents/test_repair/config.py b/agents/test-repair/hackbot_agents/test_repair/config.py new file mode 100644 index 0000000000..de14cfeb5b --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/config.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Models and tool allowlist for the test-repair agent.""" + +ANALYSIS_MODEL = "claude-opus-4-8" +FIX_MODEL = "claude-opus-4-8" + +# Bugzilla MCP tool names as exposed to the agent (mcp____). +BUGZILLA_READ_TOOLS = [ + "mcp__bugzilla__search_bugs", + "mcp__bugzilla__get_bugs", + "mcp__bugzilla__get_bug_comments", + "mcp__bugzilla__get_bug_attachments", + "mcp__bugzilla__download_attachment", +] + +# In-process Firefox build/test MCP tools (reused from agent-tools). The agent +# uses these to reproduce a test failure and verify a proposed fix. +BUILD_TOOL = "mcp__firefox__build_firefox" +FIREFOX_TOOLS = [ + BUILD_TOOL, + "mcp__firefox__bootstrap_firefox", + "mcp__firefox__evaluate_testcase", + "mcp__firefox__evaluate_js_shell", +] + +# Built-in tools the agent may call alongside the MCP servers. The agent runs in +# an isolated container (permission_mode="bypassPermissions"), and reads the +# candidate commit diffs via Bash `git show`. +ALLOWED_TOOLS = [ + "Read", + "Grep", + "Glob", + "Bash", + "Edit", + "Write", + "MultiEdit", + "WebFetch", + "WebSearch", +] + +ADDITIONAL_DIRS = [ + "~/.mozbuild", + "~/.cache/uv/", +] diff --git a/agents/test-repair/hackbot_agents/test_repair/logs.py b/agents/test-repair/hackbot_agents/test_repair/logs.py new file mode 100644 index 0000000000..bebbc6cb03 --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/logs.py @@ -0,0 +1,103 @@ +# -*- coding: utf-8 -*- +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Download and sanitize a failing test task's log. + +Before invoking Claude we fetch each failing task's latest-run +``live_backing.log`` and write two files to the scratch dir: the full log and a +sanitized companion that keeps only the interesting lines -- the test harness's +``TEST-UNEXPECTED-*`` result lines plus ``ERROR -`` / ``FATAL -`` lines. The +agent starts from the sanitized log so its context isn't drowned by tens of MB +of output, and falls back to the full log for surrounding detail. +""" + +from __future__ import annotations + +import asyncio +import logging +import re +from pathlib import Path +from typing import NamedTuple + +import requests + +logger = logging.getLogger(__name__) + +ARTIFACT_URL = ( + "https://firefox-ci-tc.services.mozilla.com/api/queue/v1/" + "task/{task_id}/artifacts/public/logs/live_backing.log" +) +_HEADERS = {"User-Agent": "hackbot-test-repair/1.0"} +_TIMEOUT = 120 +_MAX_LINES = 2000 + +_INTERESTING_RE = re.compile(r"TEST-UNEXPECTED-|(?:ERROR|FATAL) -") + + +class TaskLogs(NamedTuple): + """Paths to the two log files written for one failing task.""" + + sanitized: Path + full: Path + + +def _safe_filename(task_name: str) -> str: + return re.sub(r"[^A-Za-z0-9._-]+", "_", task_name).strip("_") or "task" + + +def sanitize_log(text: str) -> str: + """Keep only failure/error lines, deduping consecutive repeats and capping size.""" + kept: list[str] = [] + previous: str | None = None + for line in text.splitlines(): + if not _INTERESTING_RE.search(line): + continue + stripped = line.rstrip() + if stripped == previous: + continue + previous = stripped + kept.append(stripped) + if len(kept) >= _MAX_LINES: + kept.append(f"... (truncated at {_MAX_LINES} lines)") + break + return "\n".join(kept) + + +def _fetch_and_write(task_name: str, task_id: str, dest_dir: Path) -> TaskLogs: + safe = _safe_filename(task_name) + full_path = dest_dir / f"{safe}.log" + sanitized_path = dest_dir / f"{safe}.failures.txt" + url = ARTIFACT_URL.format(task_id=task_id) + try: + resp = requests.get(url, headers=_HEADERS, timeout=_TIMEOUT) + resp.raise_for_status() + full_path.write_text(resp.text) + sanitized = sanitize_log(resp.text) + sanitized_path.write_text( + sanitized if sanitized else f"(no failure lines matched in {url})\n" + ) + except requests.exceptions.RequestException as exc: + logger.warning("Failed to download log for %s (%s): %s", task_name, url, exc) + note = f"(failed to download {url}: {exc})\n" + full_path.write_text(note) + sanitized_path.write_text(note) + return TaskLogs(sanitized=sanitized_path, full=full_path) + + +async def download_failure_logs( + failure_tasks: dict[str, str], dest_dir: Path +) -> dict[str, TaskLogs]: + """Download and sanitize each task's log concurrently. + + Returns a mapping of task name to its :class:`TaskLogs`. + """ + names = list(failure_tasks) + logs = await asyncio.gather( + *( + asyncio.to_thread(_fetch_and_write, name, failure_tasks[name], dest_dir) + for name in names + ) + ) + return dict(zip(names, logs)) diff --git a/agents/test-repair/hackbot_agents/test_repair/prompts.py b/agents/test-repair/hackbot_agents/test_repair/prompts.py new file mode 100644 index 0000000000..8e20d731b3 --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/prompts.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Prompt templates for the test-repair agent.""" + +ANALYSIS_TEMPLATE = """\ +You are investigating a failing Firefox CI test to find the commit that broke it. +Treat this as a genuine regression: a commit that landed since the test was last +green introduced the failure. + +Failing test groups (manifests) and a representative failing test in each: +{failing_tests} +Test harness: {harness} + +The source tree is checked out at the failure commit {failure_commit}. The commits +that landed since this test was last green are listed below, newest first -- the +culprit is one of these: +{commit_lines} +{last_green_line} +Failure logs (start with the sanitized failures file; fall back to the full log): +{failure_logs} + +Do the following: +1. Read the sanitized failure lines to understand exactly how the test failed. +2. Use `git show ` on the candidate commits above to inspect their diffs + and identify the single commit that most plausibly introduced the failure. You + may search Bugzilla for a related bug. +3. Write these files to {scratch_out}: + - summary.md: a short (2-4 sentence) verdict. + - analysis.md: the detailed reasoning, with evidence from the logs and diffs. + - verdict.json: an object with keys "culprit_commit" (a full sha from the + candidates above, or null if none is convincing), "culprit_bug" (integer or + null), "recommendation" ("backout" or "land_fix") and "confidence" + (0.0-1.0). + +Do not edit any source files in this step. +""" + +LAST_GREEN_LINE = "The test was last green at revision {last_green_revision}.\n" + +FIX_TEMPLATE = """\ +You determined that commit {culprit_commit} regressed the failing test(s). +Propose a minimal source patch that fixes the failure. + +1. Make the smallest change that addresses the root cause you identified in + {scratch_out}/analysis.md. +2. If practical, verify the fix with the Firefox MCP tools (build_firefox / + evaluate_testcase). +3. Update {scratch_out}/verdict.json: set "proposed_patch" to true if you made a + fix, and set "recommendation" to "land_fix" only if you are confident in the + fix; otherwise keep "backout". + +Keep the patch minimal and focused on the regression. +""" diff --git a/agents/test-repair/hackbot_agents/test_repair/resolve.py b/agents/test-repair/hackbot_agents/test_repair/resolve.py new file mode 100644 index 0000000000..631c734310 --- /dev/null +++ b/agents/test-repair/hackbot_agents/test_repair/resolve.py @@ -0,0 +1,259 @@ +# -*- coding: utf-8 -*- +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Resolve a failing Taskcluster test task into everything the agent needs. + +From a task id alone, derive the push it belongs to (project + hg revision), the +test groups that failed, the revision at which the failing group was last green, +and the git commits that landed since then (head first). That commit range both +bounds the culprit search and sizes the shallow clone. The agent recomputes all +of this itself so its only input is a task id; the pulse listener uses the same +public Taskcluster / mozci / hg-pushlog / lando lookups only to decide which +failures are worth investigating. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass + +import mozci.push # noqa: F401 (imported so mozci registers its data sources) +import requests +from mozci import data +from mozci.errors import ParentPushNotFound +from mozci.push import MAX_DEPTH, Push +from mozci.task import Status + +logger = logging.getLogger(__name__) + +_TC_TASK_URL = "https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/{task_id}" +_LANDO_HG2GIT = "https://lando.moz.tools/api/hg2git/firefox/{rev}" +_HG_BASE = "https://hg.mozilla.org" +# Taskcluster ``project`` tag -> hg pushlog repository path. +_REPO_PATHS = { + "autoland": "integration/autoland", + "mozilla-central": "mozilla-central", + "mozilla-beta": "releases/mozilla-beta", + "mozilla-release": "releases/mozilla-release", + "try": "try", +} +_HEADERS = {"User-Agent": "hackbot-test-repair/1.0"} +_TIMEOUT = 30 +# Hard cap so an old last-green can't produce an unbounded clone depth. +MAX_CANDIDATES = 50 + + +@dataclass(frozen=True) +class FailingGroup: + """A failing test manifest and a representative failing test within it.""" + + group: str + test: str + + +@dataclass +class Investigation: + """The resolved context for one test-repair run, derived from a task id.""" + + project: str + hg_revision: str + harness: str + failing_groups: list[FailingGroup] + last_green_revision: str | None + # Git hashes of the commits to inspect, head (failure) commit first. + candidate_commits: list[str] + + @property + def failure_commit(self) -> str: + return self.candidate_commits[0] + + +def _get_json(url: str) -> dict: + resp = requests.get(url, headers=_HEADERS, timeout=_TIMEOUT) + resp.raise_for_status() + return resp.json() + + +def _hg_to_git(rev: str) -> str | None: + try: + return _get_json(_LANDO_HG2GIT.format(rev=rev)).get("git_hash") + except requests.exceptions.RequestException: + logger.warning("lando hg2git lookup failed for %s", rev) + return None + + +def _harness(tags: dict) -> str: + """The tests.firefox.dev harness key for a test task.""" + suite = tags.get("test-suite") or "" + label = tags.get("label") or "" + if "xpcshell" in suite or "xpcshell" in label: + return "xpcshell" + if "mochitest" in suite: + return "mochitest" + return tags.get("kind") or suite or "unknown" + + +def _failing_groups(task_id: str) -> list[FailingGroup]: + """Failing test groups for a task, via mozci; empty on any error.""" + try: + by_group = data.handler.get("test_task_failure_types", task_id=task_id) + except Exception: + logger.exception("Could not read failing groups for task %s", task_id) + return [] + groups: list[FailingGroup] = [] + for group, fails in by_group.items(): + if not group or not fails: + continue + test, _ftype = fails[0] + groups.append(FailingGroup(group=group, test=test)) + return groups + + +def _group_status(push: Push, group: str) -> str | None: + """'passed'/'failed'/None for a test group on a push (None = non-decisive).""" + summary = push.group_summaries.get(group) + if summary is None: + return None + if summary.status == Status.PASS: + return "passed" + if summary.status == Status.FAIL: + return "failed" + # INTERMITTENT or still running: can't anchor last-green here. + return None + + +def _last_green(branch: str, rev: str, group: str) -> str | None: + """Most recent ancestor revision where ``group`` was green, best effort. + + Returns None when no green ancestor is found within MAX_DEPTH, the group was + already failing upstream, or mozci errors -- the caller then falls back to the + failing push alone. + """ + try: + ancestor = Push(rev, branch=branch) + for _ in range(MAX_DEPTH): + try: + ancestor = ancestor.parent + except ParentPushNotFound: + break + status = _group_status(ancestor, group) + if status == "passed": + return ancestor.rev + if status == "failed": + return None + except Exception: + logger.exception("Could not determine last-green for %s at %s", group, rev) + return None + + +def _commits_from_pushes(pushes: dict) -> list[tuple[str | None, str | None]]: + """Flatten pushlog pushes into (hg_node, git_hash) pairs, oldest first.""" + pairs: list[tuple[str | None, str | None]] = [] + for key in sorted(pushes, key=int): + push = pushes[key] + changesets = push.get("changesets") or [] + git_changesets = push.get("git_changesets") or [] + for i, cs in enumerate(changesets): + node = cs.get("node") if isinstance(cs, dict) else cs + git = git_changesets[i] if i < len(git_changesets) else None + pairs.append((node, git)) + return pairs + + +def _candidate_commits( + project: str, + head_rev: str, + last_green_rev: str | None, + max_candidates: int, +) -> list[str]: + """Git hashes for commits in ``(last_green_rev, head_rev]``, head first. + + Falls back to the head push when there is no last-green, and to the single + head commit when the pushlog can't be read, so the result always includes the + head. Capped to ``max_candidates`` newest commits. + """ + path = _REPO_PATHS.get(project, project) + base = f"{_HG_BASE}/{path}/json-pushes" + try: + if last_green_rev: + url = ( + f"{base}?fromchange={last_green_rev}" + f"&tochange={head_rev}&full=1&version=2" + ) + else: + url = f"{base}?changeset={head_rev}&full=1&version=2" + pushes = _get_json(url).get("pushes") or {} + except requests.exceptions.RequestException: + logger.exception( + "Failed to fetch %s pushlog (%s..%s)", project, last_green_rev, head_rev + ) + pushes = {} + + git_commits: list[str] = [] + for node, git in _commits_from_pushes(pushes): + if not git and node: + git = _hg_to_git(node) + if git: + git_commits.append(git) + git_commits.reverse() # pushlog is oldest-first; we want head first. + + if not git_commits: + head_git = _hg_to_git(head_rev) + return [head_git] if head_git else [] + + if len(git_commits) > max_candidates: + logger.warning( + "Candidate range %s..%s has %d commits; capping to the newest %d", + last_green_rev, + head_rev, + len(git_commits), + max_candidates, + ) + git_commits = git_commits[:max_candidates] + return git_commits + + +def resolve_investigation( + task_id: str, *, max_candidates: int = MAX_CANDIDATES +) -> Investigation: + """Resolve a failing test task into its investigation context. + + Raises ``ValueError`` when the task has no revision (nothing to investigate). + """ + task = _get_json(_TC_TASK_URL.format(task_id=task_id)) + tags = task.get("tags") or {} + project = tags.get("project") or "autoland" + hg_revision = (task.get("payload") or {}).get("env", {}).get("GECKO_HEAD_REV") + if not hg_revision: + raise ValueError(f"task {task_id} has no GECKO_HEAD_REV") + logger.info("Resolved task %s: project=%s rev=%s", task_id, project, hg_revision) + + groups = _failing_groups(task_id) + logger.info( + "Failing groups: %s", ", ".join(g.group for g in groups) or "none resolved" + ) + + last_green = _last_green(project, hg_revision, groups[0].group) if groups else None + logger.info("Last-green revision: %s", last_green or "not found") + + candidate_commits = _candidate_commits( + project, hg_revision, last_green, max_candidates + ) + if not candidate_commits: + raise ValueError(f"could not resolve any git commit for task {task_id}") + logger.info( + "Resolved %d candidate commit(s), head %s", + len(candidate_commits), + candidate_commits[0], + ) + + return Investigation( + project=project, + hg_revision=hg_revision, + harness=_harness(tags), + failing_groups=groups, + last_green_revision=last_green, + candidate_commits=candidate_commits, + ) diff --git a/agents/test-repair/pyproject.toml b/agents/test-repair/pyproject.toml new file mode 100644 index 0000000000..34270dc3c5 --- /dev/null +++ b/agents/test-repair/pyproject.toml @@ -0,0 +1,30 @@ +[project] +name = "hackbot-agent-test-repair" +version = "0.1.0" +description = "Cloud Run Job image that runs the test-repair agent for hackbot-api" +requires-python = ">=3.12" +dependencies = [ + "hackbot-runtime[claude-sdk]", + "agent-tools[bugzilla,firefox]", + "claude-agent-sdk>=0.1.30", + "mcp>=1.0.0", + "requests", + "mozci~=2.4.8", + # mozci imports zstandard eagerly (default cache serializer) but only declares + # it under its optional `cache` extra, so pull it in explicitly. + "zstandard~=0.25.0", +] + +[tool.uv.sources] +hackbot-runtime = { workspace = true } +agent-tools = { workspace = true } + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["hackbot_agents"] + +[tool.pytest.ini_options] +testpaths = ["tests"] diff --git a/agents/test-repair/tests/__init__.py b/agents/test-repair/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/agents/test-repair/tests/test_agent_loop.py b/agents/test-repair/tests/test_agent_loop.py new file mode 100644 index 0000000000..f37c06879f --- /dev/null +++ b/agents/test-repair/tests/test_agent_loop.py @@ -0,0 +1,149 @@ +import asyncio +import json +from types import SimpleNamespace + +from hackbot_agents.test_repair import agent +from hackbot_agents.test_repair.resolve import FailingGroup, Investigation + + +def _result_msg(): + return SimpleNamespace( + is_error=False, total_cost_usd=0.1, num_turns=3, result=None, subtype=None + ) + + +def _investigation(): + return Investigation( + project="autoland", + hg_revision="hgrev", + harness="mochitest", + failing_groups=[ + FailingGroup("dom/base/test/mochitest.ini", "dom/base/test/a.js") + ], + last_green_revision="greensha", + candidate_commits=["headsha", "oldsha"], + ) + + +def _run(tmp_path, verdicts, monkeypatch): + scratch_out = tmp_path / "out" + scratch_out.mkdir() + calls = [] + + async def fake_session(reporter, options, prompt): + calls.append(prompt) + verdict = verdicts.pop(0) + (scratch_out / "verdict.json").write_text(json.dumps(verdict)) + (scratch_out / "summary.md").write_text("the verdict") + (scratch_out / "analysis.md").write_text("the reasoning") + return _result_msg() + + monkeypatch.setattr(agent, "_run_session", fake_session) + monkeypatch.setattr(agent, "build_sdk_server", lambda *a, **k: {"type": "sdk"}) + + result = asyncio.run( + agent.run_test_repair( + bugzilla_mcp_server=None, + source_repo=tmp_path, + fx_ctx=object(), + investigation=_investigation(), + task_logs={}, + scratch_out=scratch_out, + verbose=False, + log=None, + ) + ) + return result, calls + + +def test_culprit_runs_fix_stage(tmp_path, monkeypatch): + result, calls = _run( + tmp_path, + [ + { + "recommendation": "backout", + "culprit_commit": "headsha", + "confidence": 0.9, + }, + { + "recommendation": "land_fix", + "culprit_commit": "headsha", + "confidence": 0.9, + "proposed_patch": True, + }, + ], + monkeypatch, + ) + assert len(calls) == 2 # analysis + fix + assert result.classification == "regression" + assert result.culprit_commit == "headsha" + assert result.recommendation == "land_fix" + assert result.proposed_patch is True + assert result.last_green_revision == "greensha" + assert result.num_turns == 6 + + +def test_no_culprit_skips_fix_stage(tmp_path, monkeypatch): + result, calls = _run( + tmp_path, + [ + { + "recommendation": "backout", + "culprit_commit": None, + "confidence": 0.3, + }, + ], + monkeypatch, + ) + assert len(calls) == 1 # no fix stage + assert result.culprit_commit is None + assert result.proposed_patch is False + + +def test_assemble_defaults_on_missing_verdict(tmp_path): + out = tmp_path / "out" + out.mkdir() + result = agent._assemble_result( + out, + last_green_revision=None, + total_turns=1, + total_cost=0.0, + publish_file=None, + ) + # The agent assumes a regression; a missing verdict defaults accordingly. + assert result.classification == "regression" + assert result.recommendation == "backout" + + +def test_assemble_tolerates_malformed_verdict_fields(tmp_path): + # verdict.json is authored by the model; bad confidence/bug must not crash the + # run after both stages have already done the expensive work. + out = tmp_path / "out" + out.mkdir() + (out / "verdict.json").write_text( + json.dumps( + { + "recommendation": "backout", + "culprit_commit": "abc", + "confidence": "high", + "culprit_bug": "n/a", + } + ) + ) + result = agent._assemble_result( + out, + last_green_revision=None, + total_turns=1, + total_cost=0.0, + publish_file=None, + ) + assert result.confidence == 0.0 + assert result.culprit_bug is None + assert result.classification == "regression" + assert result.culprit_commit == "abc" + + +def test_coerce_recommendation_defaults_by_classification(): + assert agent._coerce_recommendation("bogus", "regression") == "backout" + assert agent._coerce_recommendation("bogus", "intermittent") == "do_not_backout" + assert agent._coerce_recommendation("land_fix", "regression") == "land_fix" diff --git a/agents/test-repair/tests/test_resolve.py b/agents/test-repair/tests/test_resolve.py new file mode 100644 index 0000000000..f40ea99b93 --- /dev/null +++ b/agents/test-repair/tests/test_resolve.py @@ -0,0 +1,117 @@ +import pytest +from hackbot_agents.test_repair import resolve +from hackbot_agents.test_repair.resolve import FailingGroup +from mozci.errors import ParentPushNotFound +from mozci.task import Status + +GROUP = "dom/base/test/mochitest.ini" + + +class FakeSummary: + def __init__(self, status): + self.status = status + + +class FakePush: + def __init__(self, rev, summaries=None, parent=None): + self.rev = rev + self.group_summaries = summaries or {} + self._parent = parent + + @property + def parent(self): + if self._parent is None: + raise ParentPushNotFound(f"no parent for {self.rev}") + return self._parent + + +def test_harness_detection(): + assert resolve._harness({"test-suite": "xpcshell"}) == "xpcshell" + assert resolve._harness({"label": "test-linux/opt-xpcshell-4"}) == "xpcshell" + assert resolve._harness({"test-suite": "mochitest-browser-chrome"}) == "mochitest" + assert resolve._harness({"kind": "web-platform-tests"}) == "web-platform-tests" + assert resolve._harness({}) == "unknown" + + +def test_last_green_returns_first_passing_ancestor(monkeypatch): + green = FakePush("greenrev", {GROUP: FakeSummary(Status.PASS)}) + flaky = FakePush( + "flakyrev", {GROUP: FakeSummary(Status.INTERMITTENT)}, parent=green + ) + head = FakePush("headrev", {}, parent=flaky) + monkeypatch.setattr(resolve, "Push", lambda rev, branch=None: head) + assert resolve._last_green("autoland", "headrev", GROUP) == "greenrev" + + +def test_last_green_none_when_already_failing_upstream(monkeypatch): + parent = FakePush("parentrev", {GROUP: FakeSummary(Status.FAIL)}) + head = FakePush("headrev", {}, parent=parent) + monkeypatch.setattr(resolve, "Push", lambda rev, branch=None: head) + assert resolve._last_green("autoland", "headrev", GROUP) is None + + +def test_last_green_fails_soft_on_error(monkeypatch): + def boom(rev, branch=None): + raise RuntimeError("mozci exploded") + + monkeypatch.setattr(resolve, "Push", boom) + assert resolve._last_green("autoland", "headrev", GROUP) is None + + +def test_candidate_commits_range_head_first(monkeypatch): + pushes = { + "1": {"changesets": [{"node": "hgA"}], "git_changesets": ["gitA"]}, + "2": { + "changesets": [{"node": "hgB"}, {"node": "hgC"}], + "git_changesets": ["gitB", "gitC"], + }, + } + monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) + commits = resolve._candidate_commits("autoland", "hgC", "hgA", 50) + assert commits == ["gitC", "gitB", "gitA"] + + +def test_candidate_commits_capped_newest_first(monkeypatch): + pushes = { + str(i): {"changesets": [{"node": f"hg{i}"}], "git_changesets": [f"git{i}"]} + for i in range(1, 4) + } + monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) + commits = resolve._candidate_commits("autoland", "hg3", "hg0", 2) + assert commits == ["git3", "git2"] + + +def test_candidate_commits_falls_back_to_head_commit(monkeypatch): + def boom(url): + raise resolve.requests.exceptions.RequestException("hg down") + + monkeypatch.setattr(resolve, "_get_json", boom) + monkeypatch.setattr(resolve, "_hg_to_git", lambda rev: "gitHEAD") + assert resolve._candidate_commits("autoland", "hgHEAD", "hgOLD", 50) == ["gitHEAD"] + + +def test_resolve_investigation_assembles_context(monkeypatch): + task = { + "tags": {"project": "autoland", "test-suite": "mochitest-browser-chrome"}, + "payload": {"env": {"GECKO_HEAD_REV": "hghead"}}, + } + monkeypatch.setattr(resolve, "_get_json", lambda url: task) + monkeypatch.setattr( + resolve, "_failing_groups", lambda tid: [FailingGroup(GROUP, "a.js")] + ) + monkeypatch.setattr(resolve, "_last_green", lambda *a: "greenrev") + monkeypatch.setattr(resolve, "_candidate_commits", lambda *a: ["gitHead", "gitOld"]) + + inv = resolve.resolve_investigation("TASK") + assert inv.project == "autoland" + assert inv.hg_revision == "hghead" + assert inv.harness == "mochitest" + assert inv.last_green_revision == "greenrev" + assert inv.candidate_commits == ["gitHead", "gitOld"] + assert inv.failure_commit == "gitHead" + + +def test_resolve_investigation_requires_revision(monkeypatch): + monkeypatch.setattr(resolve, "_get_json", lambda url: {"tags": {}, "payload": {}}) + with pytest.raises(ValueError): + resolve.resolve_investigation("TASK") diff --git a/agents/test-repair/tests/test_scaffold.py b/agents/test-repair/tests/test_scaffold.py new file mode 100644 index 0000000000..f7d0248df4 --- /dev/null +++ b/agents/test-repair/tests/test_scaffold.py @@ -0,0 +1,67 @@ +import os + +from hackbot_agents.test_repair import logs +from hackbot_agents.test_repair.__main__ import _pin_checkout +from hackbot_agents.test_repair.agent import TestRepairResult + + +def test_sanitize_log_keeps_failure_and_error_lines(): + raw = "\n".join( + [ + "INFO - starting test", + "TEST-UNEXPECTED-FAIL | dom/test_a.js | assertion failed", + "some noise", + "12:00 ERROR - linker error", + "TEST-PASS | dom/test_b.js", + "FATAL - crash", + ] + ) + out = logs.sanitize_log(raw).splitlines() + assert any("TEST-UNEXPECTED-FAIL" in line for line in out) + assert any("ERROR - linker error" in line for line in out) + assert any("FATAL - crash" in line for line in out) + # Passing/info lines are dropped. + assert all("TEST-PASS" not in line for line in out) + assert all("starting test" not in line for line in out) + + +def test_sanitize_log_dedupes_consecutive_repeats(): + raw = "\n".join(["TEST-UNEXPECTED-FAIL | x | boom"] * 3) + assert len(logs.sanitize_log(raw).splitlines()) == 1 + + +def test_pin_checkout_sets_ref_and_depth(monkeypatch): + monkeypatch.delenv("SOURCE_REF", raising=False) + monkeypatch.delenv("SOURCE_DEPTH", raising=False) + _pin_checkout(["headsha", "midsha", "oldsha"]) + assert os.environ["SOURCE_REF"] == "headsha" + # Depth spans the 3 candidates plus one so the last-green parent is reachable. + assert os.environ["SOURCE_DEPTH"] == "4" + + +def test_result_model_serializes_findings(): + result = TestRepairResult( + num_turns=5, + total_cost_usd=0.42, + classification="regression", + recommendation="backout", + culprit_commit="deadbeef", + confidence=0.8, + summary="broke test", + analysis="the diff removed a null check", + ) + findings = result.model_dump() + assert findings["classification"] == "regression" + assert findings["recommendation"] == "backout" + assert findings["culprit_commit"] == "deadbeef" + assert findings["proposed_patch"] is False + + +def test_intermittent_result_defaults(): + result = TestRepairResult( + num_turns=2, + classification="intermittent", + recommendation="do_not_backout", + ) + assert result.culprit_commit is None + assert result.proposed_patch is False diff --git a/docker-compose.yml b/docker-compose.yml index 7c804a95d4..e0e6456390 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,11 +3,12 @@ version: "3.8" include: - - path: agents/autowebcompat-repro/compose.yml - - path: agents/bug-fix/compose.yml + # - path: agents/autowebcompat-repro/compose.yml + # - path: agents/bug-fix/compose.yml - path: agents/build-repair/compose.yml - - path: agents/frontend-triage/compose.yml - - path: agents/test-plan-generator/compose.yml + - path: agents/test-repair/compose.yml +# - path: agents/frontend-triage/compose.yml +# - path: agents/test-plan-generator/compose.yml services: bugbug-base: diff --git a/services/hackbot-api/app/agents.py b/services/hackbot-api/app/agents.py index eb5aa804bb..0ea62cecfc 100644 --- a/services/hackbot-api/app/agents.py +++ b/services/hackbot-api/app/agents.py @@ -10,6 +10,7 @@ BuildRepairInputs, FrontendTriageInputs, TestPlanGeneratorInputs, + TestRepairInputs, ) @@ -79,6 +80,16 @@ def model_to_env(inputs: BaseModel) -> dict[str, str]: job_name="hackbot-agent-frontend-triage", input_schema=FrontendTriageInputs, ), + "test-repair": AgentSpec( + name="test-repair", + description=( + "Analyze a Firefox CI test failure: classify it as a regression or an " + "intermittent, blame the culprit commit, and propose a fix patch for " + "regressions." + ), + job_name="hackbot-agent-test-repair", + input_schema=TestRepairInputs, + ), "test-plan-generator": AgentSpec( name="test-plan-generator", description=( diff --git a/services/hackbot-api/app/schemas.py b/services/hackbot-api/app/schemas.py index 5e3d85751a..bf1327463e 100644 --- a/services/hackbot-api/app/schemas.py +++ b/services/hackbot-api/app/schemas.py @@ -107,6 +107,15 @@ class BuildRepairInputs(BaseModel): max_turns: int | None = None +class TestRepairInputs(BaseModel): + # Failing Taskcluster test tasks {task_name: task_id}. The agent resolves the + # push, the last-green revision and the candidate commit range itself from the + # task id (the listener only filters which failures are worth investigating). + failure_tasks: dict[str, str] + model: str | None = None + max_turns: int | None = None + + class FrontendTriageInputs(BaseModel): bug_id: int model: str | None = None diff --git a/services/hackbot-api/tests/test_agents.py b/services/hackbot-api/tests/test_agents.py index 1e782b9963..aa14ec6c5b 100644 --- a/services/hackbot-api/tests/test_agents.py +++ b/services/hackbot-api/tests/test_agents.py @@ -7,6 +7,7 @@ from app.schemas import ( BugFixInputs, BuildRepairInputs, + TestRepairInputs, ) from app.schemas import ( TestPlanGeneratorInputs as PlanGeneratorInputs, @@ -91,3 +92,29 @@ def test_test_plan_generator_registry_uses_default_env_serializer(): assert spec.build_env is None assert spec.job_name == "hackbot-agent-test-plan-generator" assert spec.input_schema is PlanGeneratorInputs + + +def test_test_repair_registry_entry(): + spec = AGENT_REGISTRY["test-repair"] + assert spec.build_env is None + assert spec.input_schema is TestRepairInputs + assert spec.job_name == "hackbot-agent-test-repair" + assert spec.auto_apply_actions is False + + +def test_test_repair_env_serialization(): + # The agent resolves the test, commit range and clone depth from the task id, + # so the only per-run input is the failing task mapping. + env = model_to_env( + TestRepairInputs( + failure_tasks={"test-linux1804-64/opt-xpcshell-1": "abc123"}, + ) + ) + assert json.loads(env["FAILURE_TASKS"]) == { + "test-linux1804-64/opt-xpcshell-1": "abc123" + } + + +def test_test_repair_inputs_require_failure_tasks(): + with pytest.raises(ValidationError): + TestRepairInputs(model="claude-opus-4-8") diff --git a/uv.lock b/uv.lock index 2024e35395..40273f1a49 100644 --- a/uv.lock +++ b/uv.lock @@ -27,6 +27,7 @@ members = [ "hackbot-agent-build-repair", "hackbot-agent-frontend-triage", "hackbot-agent-test-plan-generator", + "hackbot-agent-test-repair", "hackbot-api", "hackbot-pulse-listener", "hackbot-runtime", @@ -2607,6 +2608,31 @@ requires-dist = [ { name = "mozinstall" }, ] +[[package]] +name = "hackbot-agent-test-repair" +version = "0.1.0" +source = { editable = "agents/test-repair" } +dependencies = [ + { name = "agent-tools", extra = ["bugzilla", "firefox"] }, + { name = "claude-agent-sdk" }, + { name = "hackbot-runtime", extra = ["claude-sdk"] }, + { name = "mcp" }, + { name = "mozci" }, + { name = "requests" }, + { name = "zstandard" }, +] + +[package.metadata] +requires-dist = [ + { name = "agent-tools", extras = ["bugzilla", "firefox"], editable = "libs/agent-tools" }, + { name = "claude-agent-sdk", specifier = ">=0.1.30" }, + { name = "hackbot-runtime", extras = ["claude-sdk"], editable = "libs/hackbot-runtime" }, + { name = "mcp", specifier = ">=1.0.0" }, + { name = "mozci", specifier = "~=2.4.8" }, + { name = "requests" }, + { name = "zstandard", specifier = "~=0.25.0" }, +] + [[package]] name = "hackbot-api" version = "0.1.0" From 7bfb6731444b06383f65813a19bc17da75088a99 Mon Sep 17 00:00:00 2001 From: Evgeny Pavlov Date: Thu, 23 Jul 2026 15:26:32 -0700 Subject: [PATCH 2/3] Uncomment other compose files --- docker-compose.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index e0e6456390..76e3b4c1ca 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,12 +3,12 @@ version: "3.8" include: - # - path: agents/autowebcompat-repro/compose.yml - # - path: agents/bug-fix/compose.yml + - path: agents/autowebcompat-repro/compose.yml + - path: agents/bug-fix/compose.yml - path: agents/build-repair/compose.yml - path: agents/test-repair/compose.yml -# - path: agents/frontend-triage/compose.yml -# - path: agents/test-plan-generator/compose.yml + - path: agents/frontend-triage/compose.yml + - path: agents/test-plan-generator/compose.yml services: bugbug-base: From 5d3994d4980c802813ad1c07935f9f5f397136cc Mon Sep 17 00:00:00 2001 From: Evgeny Pavlov Date: Fri, 24 Jul 2026 16:25:21 -0700 Subject: [PATCH 3/3] Correctness fixes --- agents/test-repair/README.md | 32 ++- .../hackbot_agents/test_repair/__main__.py | 13 +- .../hackbot_agents/test_repair/agent.py | 127 ++++++++-- .../hackbot_agents/test_repair/prompts.py | 53 ++-- .../hackbot_agents/test_repair/resolve.py | 143 ++++++----- agents/test-repair/tests/test_agent_loop.py | 239 ++++++++++++++---- agents/test-repair/tests/test_resolve.py | 118 +++++++-- agents/test-repair/tests/test_scaffold.py | 18 +- .../hackbot-ui/components/TriggerForm.tsx | 93 ++++--- services/hackbot-ui/lib/agents.ts | 1 + 10 files changed, 603 insertions(+), 234 deletions(-) diff --git a/agents/test-repair/README.md b/agents/test-repair/README.md index c4b9ac133e..17f431b5f9 100644 --- a/agents/test-repair/README.md +++ b/agents/test-repair/README.md @@ -4,8 +4,8 @@ Two-stage Claude agent that finds the commit which regressed a failing Firefox C test and proposes a fix. Agent logic in `hackbot_agents/test_repair/`. The pulse listener only forwards failures that already passed its regression and -flakiness filters, so the agent assumes a genuine regression and does not -re-classify. Its only input is a Taskcluster task id. +flakiness filters, so a regression is the prior, but the agent still reports the +classification it reaches. Its only input is a Taskcluster task id. Run the Docker command below from the repo root, with secrets in a local `.env` (`ANTHROPIC_API_KEY`; `BUGZILLA_API_KEY` is optional). @@ -19,19 +19,24 @@ investigation needs (no log parsing): 2. The failing test groups, via mozci. 3. The revision at which the group was last green, by walking mozci push ancestors. -4. The git commits that landed since then (head first) from the hg pushlog + - lando; capped so an old last-green can't produce an unbounded clone. +4. The git range that landed since then, from the hg pushlog + lando. Only the + range endpoints are mapped to git; the commit count sizes the clone and is + capped so an old last-green can't produce an unbounded one. -The head (failure) commit and a depth spanning back to last-green are set as -`SOURCE_REF` / `SOURCE_DEPTH`, so the runtime shallow-clones exactly deep enough -for the agent to `git show` every candidate. The task's full and sanitized logs -are written to files for the agent to search. +The agent gets the range (`base..head`), not a list of shas, and enumerates and +narrows it itself with `git log`. When the range isn't known to reach a green run +it is passed as `HEAD~N..HEAD` and the prompt stops asserting the culprit is in +it. + +`SOURCE_REF` / `SOURCE_DEPTH` pin the shallow clone to the failure commit, deep +enough to walk the range. The task's full and sanitized logs are written to files +for the agent to search. ## Input - `FAILURE_TASKS` - a dictionary of failed Taskcluster test tasks - `{task_name: taskcluster_task_id}`. The agent resolves the push, last-green - revision and candidate commit range from the first task id itself. + `{task_name: taskcluster_task_id}`. Everything else is resolved from the first + task id. ## Output @@ -39,10 +44,11 @@ First stage - analysis (read-only): - `summary.md` - a short verdict - `analysis.md` - detailed reasoning, with evidence from the logs and diffs -- `verdict.json` - `culprit_commit`, `culprit_bug`, `recommendation` - (`backout` / `land_fix`) and `confidence` +- `verdict.json` - `classification` (`regression` / `intermittent`), + `culprit_commit`, `culprit_bug`, `intermittent_bug`, `recommendation` + (`backout` / `land_fix` / `do_not_backout`) and `confidence` -Second stage - fixing (only when a culprit is identified): +Second stage - fixing (only when a culprit was identified): - A patch in Hackbot format diff --git a/agents/test-repair/hackbot_agents/test_repair/__main__.py b/agents/test-repair/hackbot_agents/test_repair/__main__.py index 04f48d5ecb..85d763dc38 100644 --- a/agents/test-repair/hackbot_agents/test_repair/__main__.py +++ b/agents/test-repair/hackbot_agents/test_repair/__main__.py @@ -25,15 +25,14 @@ class AgentInputs(BaseSettings): model_config = SettingsConfigDict(extra="ignore", env_ignore_empty=True) -def _pin_checkout(candidate_commits: list[str]) -> None: +def _pin_checkout(investigation: Investigation) -> None: """Pin the shallow clone to the failure commit, deep enough for the range. - ``SOURCE_REF`` is the head (failure) commit and ``SOURCE_DEPTH`` spans back to - the last-green commit so the agent can ``git show`` every candidate. Read by - the runtime when it prepares the source tree (HackbotContext.source_repo). + Read by the runtime when it prepares the source tree + (HackbotContext.source_repo). """ - os.environ.setdefault("SOURCE_REF", candidate_commits[0]) - os.environ.setdefault("SOURCE_DEPTH", str(len(candidate_commits) + 1)) + os.environ.setdefault("SOURCE_REF", investigation.failure_commit) + os.environ.setdefault("SOURCE_DEPTH", str(investigation.commit_range.span + 1)) logger.info( "Pinning checkout to %s with depth %s", os.environ["SOURCE_REF"], @@ -51,7 +50,7 @@ async def main(ctx: HackbotContext) -> TestRepairResult: task_id = next(iter(inputs.failure_tasks.values())) logger.info("Starting test-repair for task %s", task_id) investigation: Investigation = resolve_investigation(task_id) - _pin_checkout(investigation.candidate_commits) + _pin_checkout(investigation) scratch_dir = Path(tempfile.mkdtemp(prefix="test-repair-")) scratch_in = scratch_dir / "in" diff --git a/agents/test-repair/hackbot_agents/test_repair/agent.py b/agents/test-repair/hackbot_agents/test_repair/agent.py index 31ab1b62fb..67470bc0e1 100644 --- a/agents/test-repair/hackbot_agents/test_repair/agent.py +++ b/agents/test-repair/hackbot_agents/test_repair/agent.py @@ -7,18 +7,20 @@ Blame the commit that regressed a failing test and propose a fix. The pulse listener only forwards failures that already passed its regression and flakiness -filters, so the agent assumes a genuine regression and does not re-classify. +filters, so a regression is the prior, but the agent still reports the +classification it reaches from the logs. A two-stage claude-agent-sdk loop. Stage 1 (analysis, read-only) inspects the candidate commit diffs and writes a verdict naming the culprit; Stage 2 (fix) runs when a culprit is found and proposes a source patch, which the runtime -collects into ``changes.patch``. The :class:`TestRepairResult` is -serialized into ``summary.json``'s ``findings`` and read by the notifier. +collects into ``changes.patch``. The :class:`TestRepairResult` is serialized into +``summary.json``'s ``findings`` and read by the notifier. """ from __future__ import annotations import json +import subprocess import sys from collections.abc import Callable from pathlib import Path @@ -47,10 +49,12 @@ from .logs import TaskLogs from .prompts import ( ANALYSIS_TEMPLATE, + CANDIDATE_INTRO_COMPLETE, + CANDIDATE_INTRO_PARTIAL, FIX_TEMPLATE, LAST_GREEN_LINE, ) -from .resolve import FailingGroup, Investigation +from .resolve import CommitRange, FailingGroup, Investigation _CLASSIFICATIONS = ("regression", "intermittent") _RECOMMENDATIONS = ("backout", "do_not_backout", "land_fix") @@ -144,10 +148,63 @@ def _coerce_classification(value) -> str: return value if value in _CLASSIFICATIONS else "regression" -def _coerce_recommendation(value, classification: str) -> str: - if value in _RECOMMENDATIONS: - return value - return "backout" if classification == "regression" else "do_not_backout" +def _coerce_recommendation(value, classification: str, has_culprit: bool) -> str: + if value not in _RECOMMENDATIONS: + value = "backout" if classification == "regression" else "do_not_backout" + if value == "backout" and not has_culprit: + return "do_not_backout" + return value + + +def _resolve_culprit(source_repo: Path, sha) -> str | None: + """Normalize a model-authored sha to a full commit hash in the checkout. + + The shallow clone holds exactly the candidate range, so a sha git cannot + resolve there was invented or is out of range; either way it is dropped. + """ + if not isinstance(sha, str) or not sha.strip(): + return None + sha = sha.strip() + try: + proc = subprocess.run( + [ + "git", + "-C", + str(source_repo), + "rev-parse", + "--verify", + f"{sha}^{{commit}}", + ], + capture_output=True, + text=True, + ) + full = proc.stdout.strip() if proc.returncode == 0 else "" + except OSError: + full = "" + if not full: + print(f"[test-repair] discarding culprit {sha!r}", file=sys.stderr) + return None + return full + + +def _write_mozconfig(fx_ctx: FirefoxContext, *, debug: bool) -> None: + """Write a mozconfig mirroring the failing CI build, unless one exists. + + ``build_firefox`` fails outright without one. The build type is mirrored + because assertion failures only reproduce in the type CI used. + """ + if fx_ctx.mozconfig.exists(): + return + build_type = ( + "ac_add_options --enable-debug\n" + if debug + else "ac_add_options --disable-debug\nac_add_options --enable-optimize\n" + ) + fx_ctx.mozconfig.write_text( + "ac_add_options --enable-application=browser\n" + f"{build_type}" + f"mk_add_options MOZ_OBJDIR={fx_ctx.objdir}\n" + ) def _as_float(value, default: float = 0.0) -> float: @@ -167,22 +224,24 @@ def _as_int(value) -> int | None: def _assemble_result( scratch_out: Path, *, + verdict: dict, + source_repo: Path, last_green_revision: str | None, total_turns: int, total_cost: float, publish_file: Callable[[str, Path, str | None], str] | None, ) -> TestRepairResult: - verdict = _read_verdict(scratch_out) classification = _coerce_classification(verdict.get("classification")) + culprit_commit = _resolve_culprit(source_repo, verdict.get("culprit_commit")) recommendation = _coerce_recommendation( - verdict.get("recommendation"), classification + verdict.get("recommendation"), classification, bool(culprit_commit) ) - culprit_commit = verdict.get("culprit_commit") return TestRepairResult( classification=classification, recommendation=recommendation, - culprit_commit=culprit_commit or None, + culprit_commit=culprit_commit, culprit_bug=_as_int(verdict.get("culprit_bug")), + intermittent_bug=_as_int(verdict.get("intermittent_bug")), confidence=_as_float(verdict.get("confidence")), last_green_revision=last_green_revision, proposed_patch=bool(verdict.get("proposed_patch")), @@ -193,8 +252,11 @@ def _assemble_result( ) -def _commit_lines(candidate_commits: list[str]) -> str: - return "\n".join(f"- {c}" for c in candidate_commits) +def _range_expr(commit_range: CommitRange) -> str: + """A git revision range for ``git log``, falling back to the clone depth.""" + if commit_range.base: + return f"{commit_range.base}..{commit_range.head}" + return f"HEAD~{commit_range.span}..HEAD" def _failing_tests(groups: list[FailingGroup]) -> str: @@ -218,9 +280,7 @@ async def run_test_repair( publish_file: Callable[[str, Path, str | None], str] | None = None, ) -> TestRepairResult: """Blame the commit that regressed a failing test and propose a fix.""" - candidate_commits = investigation.candidate_commits - if not candidate_commits: - raise AgentError("candidate_commits must contain at least the head commit") + commit_range = investigation.commit_range failure_commit = investigation.failure_commit print( f"[test-repair] analyzing {investigation.hg_revision} at {failure_commit}", @@ -246,11 +306,16 @@ async def run_test_repair( if investigation.last_green_revision else "" ) + range_expr = _range_expr(commit_range) + intro = ( + CANDIDATE_INTRO_COMPLETE if commit_range.complete else CANDIDATE_INTRO_PARTIAL + ) analysis_prompt = ANALYSIS_TEMPLATE.format( failing_tests=_failing_tests(investigation.failing_groups), harness=investigation.harness, failure_commit=failure_commit, - commit_lines=_commit_lines(candidate_commits), + candidate_intro=intro.format(commit_range=range_expr, span=commit_range.span), + commit_range=range_expr, last_green_line=last_green_line, failure_logs=failure_logs, scratch_out=scratch_out, @@ -281,11 +346,12 @@ async def run_test_repair( total_cost += result_msg.total_cost_usd or 0.0 total_turns += result_msg.num_turns or 0 - # Stage 2 (fix) runs when the analysis identified a culprit commit. + # Stage 2 (fix) runs only when the analysis blamed a real commit. verdict = _read_verdict(scratch_out) - culprit_commit = verdict.get("culprit_commit") + culprit_commit = _resolve_culprit(source_repo, verdict.get("culprit_commit")) if culprit_commit: reporter.header(f"{label}: fix") + _write_mozconfig(fx_ctx, debug=investigation.debug_build) fix_prompt = FIX_TEMPLATE.format( culprit_commit=culprit_commit, scratch_out=scratch_out, @@ -299,13 +365,26 @@ async def run_test_repair( allowed_tools=allowed_tools, max_turns=max_turns, ) - result_msg = await _run_session(reporter, fix_opts, fix_prompt) - _check(result_msg, "fix") - total_cost += result_msg.total_cost_usd or 0.0 - total_turns += result_msg.num_turns or 0 + # A failed fix stage must not discard the analysis we already paid for. + try: + result_msg = await _run_session(reporter, fix_opts, fix_prompt) + if result_msg is not None: + total_cost += result_msg.total_cost_usd or 0.0 + total_turns += result_msg.num_turns or 0 + _check(result_msg, "fix") + except Exception as exc: + print(f"[test-repair] fix stage failed: {exc}", file=sys.stderr) + # Merge, since the fix stage may rewrite verdict.json without the culprit. + fix_verdict = _read_verdict(scratch_out) + verdict = { + **verdict, + **{k: v for k, v in fix_verdict.items() if v is not None}, + } return _assemble_result( scratch_out, + verdict=verdict, + source_repo=source_repo, last_green_revision=investigation.last_green_revision, total_turns=total_turns, total_cost=total_cost, diff --git a/agents/test-repair/hackbot_agents/test_repair/prompts.py b/agents/test-repair/hackbot_agents/test_repair/prompts.py index 8e20d731b3..23f0259c42 100644 --- a/agents/test-repair/hackbot_agents/test_repair/prompts.py +++ b/agents/test-repair/hackbot_agents/test_repair/prompts.py @@ -7,37 +7,55 @@ ANALYSIS_TEMPLATE = """\ You are investigating a failing Firefox CI test to find the commit that broke it. -Treat this as a genuine regression: a commit that landed since the test was last -green introduced the failure. +The CI listener already filtered out known intermittents, so treat this as a +genuine regression unless the logs clearly show otherwise. Failing test groups (manifests) and a representative failing test in each: {failing_tests} Test harness: {harness} -The source tree is checked out at the failure commit {failure_commit}. The commits -that landed since this test was last green are listed below, newest first -- the -culprit is one of these: -{commit_lines} +The source tree is checked out at the failure commit {failure_commit}. +{candidate_intro} {last_green_line} Failure logs (start with the sanitized failures file; fall back to the full log): {failure_logs} Do the following: 1. Read the sanitized failure lines to understand exactly how the test failed. -2. Use `git show ` on the candidate commits above to inspect their diffs - and identify the single commit that most plausibly introduced the failure. You - may search Bugzilla for a related bug. +2. Enumerate the candidates with `git log --oneline {commit_range}`. Narrow them + before reading diffs -- `git log --oneline {commit_range} -- ` on the + failing test's directory and on the source it exercises is usually enough to + get to a handful. Then `git show ` those to identify the single commit + that most plausibly introduced the failure. You may search Bugzilla for a + related bug. 3. Write these files to {scratch_out}: - summary.md: a short (2-4 sentence) verdict. - analysis.md: the detailed reasoning, with evidence from the logs and diffs. - - verdict.json: an object with keys "culprit_commit" (a full sha from the - candidates above, or null if none is convincing), "culprit_bug" (integer or - null), "recommendation" ("backout" or "land_fix") and "confidence" - (0.0-1.0). + - verdict.json: an object with keys "classification" ("regression" or + "intermittent"), "culprit_commit" (a full sha copied verbatim from the + candidate list above, or null if none is convincing), "culprit_bug" (integer + or null), "intermittent_bug" (integer or null; the bug already tracking this + intermittent, if you found one), "recommendation" ("backout", "land_fix" or + "do_not_backout") and "confidence" (0.0-1.0). + +Never guess a sha: one that is not a real commit in the range above is discarded, +and a wrong blame is worse than no blame. Use null when nothing convinces you. Do not edit any source files in this step. """ +# Picked by resolve.CommitRange.complete: how firmly the prompt may assert that +# the culprit is inside the range. +CANDIDATE_INTRO_COMPLETE = """\ +The {span} commits in `{commit_range}` landed since this test was last green -- +the culprit is one of them.""" + +CANDIDATE_INTRO_PARTIAL = """\ +`{commit_range}` is the {span} most recent commits. This range is NOT known to +reach back to a run where the test was green, so the culprit may predate it -- if +nothing in it plausibly caused the failure, say so in your analysis and set +"culprit_commit" to null.""" + LAST_GREEN_LINE = "The test was last green at revision {last_green_revision}.\n" FIX_TEMPLATE = """\ @@ -47,10 +65,11 @@ 1. Make the smallest change that addresses the root cause you identified in {scratch_out}/analysis.md. 2. If practical, verify the fix with the Firefox MCP tools (build_firefox / - evaluate_testcase). -3. Update {scratch_out}/verdict.json: set "proposed_patch" to true if you made a - fix, and set "recommendation" to "land_fix" only if you are confident in the - fix; otherwise keep "backout". + evaluate_testcase). A mozconfig matching the failing CI build is already in + place. +3. Update {scratch_out}/verdict.json in place, preserving every key it already + has: set "proposed_patch" to true if you made a fix, and set "recommendation" + to "land_fix" only if you are confident in the fix; otherwise keep "backout". Keep the patch minimal and focused on the regression. """ diff --git a/agents/test-repair/hackbot_agents/test_repair/resolve.py b/agents/test-repair/hackbot_agents/test_repair/resolve.py index 631c734310..a5b9214e1e 100644 --- a/agents/test-repair/hackbot_agents/test_repair/resolve.py +++ b/agents/test-repair/hackbot_agents/test_repair/resolve.py @@ -7,8 +7,9 @@ From a task id alone, derive the push it belongs to (project + hg revision), the test groups that failed, the revision at which the failing group was last green, -and the git commits that landed since then (head first). That commit range both -bounds the culprit search and sizes the shallow clone. The agent recomputes all +and the git range that landed since then. Only the range endpoints are mapped to +git -- the agent enumerates the commits between them with ``git log`` in the +checkout. The agent recomputes all of this itself so its only input is a task id; the pulse listener uses the same public Taskcluster / mozci / hg-pushlog / lando lookups only to decide which failures are worth investigating. @@ -41,8 +42,11 @@ } _HEADERS = {"User-Agent": "hackbot-test-repair/1.0"} _TIMEOUT = 30 -# Hard cap so an old last-green can't produce an unbounded clone depth. -MAX_CANDIDATES = 50 +# Ancestor pushes to walk looking for a green run. Each step is a live, uncached +# group_summaries lookup, so raising this trades startup latency for a better range. +LAST_GREEN_MAX_DEPTH = MAX_DEPTH +# Cap on commits (not pushes) in the range, bounding the clone depth. +MAX_RANGE_COMMITS = 100 @dataclass(frozen=True) @@ -53,6 +57,20 @@ class FailingGroup: test: str +@dataclass(frozen=True) +class CommitRange: + """The git commit range to search for the culprit.""" + + # The failure commit; what the checkout is pinned to. + head: str + # The last-green commit, exclusive. None when unknown or outside the cap. + base: str | None + # Commits in the range; bounds the shallow clone depth. + span: int + # Whether the culprit is provably inside the range. + complete: bool + + @dataclass class Investigation: """The resolved context for one test-repair run, derived from a task id.""" @@ -60,14 +78,14 @@ class Investigation: project: str hg_revision: str harness: str + debug_build: bool failing_groups: list[FailingGroup] last_green_revision: str | None - # Git hashes of the commits to inspect, head (failure) commit first. - candidate_commits: list[str] + commit_range: CommitRange @property def failure_commit(self) -> str: - return self.candidate_commits[0] + return self.commit_range.head def _get_json(url: str) -> dict: @@ -92,7 +110,13 @@ def _harness(tags: dict) -> str: return "xpcshell" if "mochitest" in suite: return "mochitest" - return tags.get("kind") or suite or "unknown" + # ``kind`` is "test" for every Firefox test task, so it is a last resort. + return suite or tags.get("kind") or "unknown" + + +def _is_debug_build(tags: dict) -> bool: + """Whether the failing task ran against a debug build.""" + return "debug" in (tags.get("test-platform") or tags.get("label") or "") def _failing_groups(task_id: str) -> list[FailingGroup]: @@ -124,16 +148,17 @@ def _group_status(push: Push, group: str) -> str | None: return None -def _last_green(branch: str, rev: str, group: str) -> str | None: +def _last_green( + branch: str, rev: str, group: str, max_depth: int = LAST_GREEN_MAX_DEPTH +) -> str | None: """Most recent ancestor revision where ``group`` was green, best effort. - Returns None when no green ancestor is found within MAX_DEPTH, the group was - already failing upstream, or mozci errors -- the caller then falls back to the - failing push alone. + None when no green ancestor is found within ``max_depth``, the group was + already failing upstream, or mozci errors. """ try: ancestor = Push(rev, branch=branch) - for _ in range(MAX_DEPTH): + for _ in range(max_depth): try: ancestor = ancestor.parent except ParentPushNotFound: @@ -148,42 +173,38 @@ def _last_green(branch: str, rev: str, group: str) -> str | None: return None -def _commits_from_pushes(pushes: dict) -> list[tuple[str | None, str | None]]: - """Flatten pushlog pushes into (hg_node, git_hash) pairs, oldest first.""" - pairs: list[tuple[str | None, str | None]] = [] - for key in sorted(pushes, key=int): - push = pushes[key] - changesets = push.get("changesets") or [] - git_changesets = push.get("git_changesets") or [] - for i, cs in enumerate(changesets): - node = cs.get("node") if isinstance(cs, dict) else cs - git = git_changesets[i] if i < len(git_changesets) else None - pairs.append((node, git)) - return pairs +def _count_commits(pushes: dict) -> int: + """Total changesets across the pushlog pushes.""" + return sum(len(p.get("changesets") or []) for p in pushes.values()) -def _candidate_commits( +def _resolve_range( project: str, head_rev: str, last_green_rev: str | None, - max_candidates: int, -) -> list[str]: - """Git hashes for commits in ``(last_green_rev, head_rev]``, head first. + max_commits: int, +) -> CommitRange | None: + """Resolve ``(last_green_rev, head_rev]`` into git endpoints and a commit count. - Falls back to the head push when there is no last-green, and to the single - head commit when the pushlog can't be read, so the result always includes the - head. Capped to ``max_candidates`` newest commits. + None when the head cannot be mapped: pinning the checkout to an older commit + would blame the wrong change. ``base`` is dropped when unknown or outside the + capped clone depth, which also marks the range incomplete. """ + head_git = _hg_to_git(head_rev) + if not head_git: + logger.error("Could not resolve a git hash for head revision %s", head_rev) + return None + path = _REPO_PATHS.get(project, project) - base = f"{_HG_BASE}/{path}/json-pushes" + pushlog = f"{_HG_BASE}/{path}/json-pushes" try: if last_green_rev: url = ( - f"{base}?fromchange={last_green_rev}" + f"{pushlog}?fromchange={last_green_rev}" f"&tochange={head_rev}&full=1&version=2" ) else: - url = f"{base}?changeset={head_rev}&full=1&version=2" + url = f"{pushlog}?changeset={head_rev}&full=1&version=2" pushes = _get_json(url).get("pushes") or {} except requests.exceptions.RequestException: logger.exception( @@ -191,32 +212,29 @@ def _candidate_commits( ) pushes = {} - git_commits: list[str] = [] - for node, git in _commits_from_pushes(pushes): - if not git and node: - git = _hg_to_git(node) - if git: - git_commits.append(git) - git_commits.reverse() # pushlog is oldest-first; we want head first. - - if not git_commits: - head_git = _hg_to_git(head_rev) - return [head_git] if head_git else [] + span = max(_count_commits(pushes), 1) + if not last_green_rev or not pushes: + return CommitRange(head_git, None, span, False) - if len(git_commits) > max_candidates: + if span > max_commits: logger.warning( - "Candidate range %s..%s has %d commits; capping to the newest %d", + "Range %s..%s has %d commits; capping the clone to the newest %d", last_green_rev, head_rev, - len(git_commits), - max_candidates, + span, + max_commits, ) - git_commits = git_commits[:max_candidates] - return git_commits + return CommitRange(head_git, None, max_commits, False) + + base_git = _hg_to_git(last_green_rev) + if not base_git: + logger.warning("Could not resolve a git hash for last-green %s", last_green_rev) + return CommitRange(head_git, None, span, False) + return CommitRange(head_git, base_git, span, True) def resolve_investigation( - task_id: str, *, max_candidates: int = MAX_CANDIDATES + task_id: str, *, max_commits: int = MAX_RANGE_COMMITS ) -> Investigation: """Resolve a failing test task into its investigation context. @@ -238,22 +256,23 @@ def resolve_investigation( last_green = _last_green(project, hg_revision, groups[0].group) if groups else None logger.info("Last-green revision: %s", last_green or "not found") - candidate_commits = _candidate_commits( - project, hg_revision, last_green, max_candidates - ) - if not candidate_commits: - raise ValueError(f"could not resolve any git commit for task {task_id}") + commit_range = _resolve_range(project, hg_revision, last_green, max_commits) + if commit_range is None: + raise ValueError(f"could not resolve a git commit for task {task_id}") logger.info( - "Resolved %d candidate commit(s), head %s", - len(candidate_commits), - candidate_commits[0], + "Range %s..%s spans %d commit(s), complete: %s", + commit_range.base or "(unknown)", + commit_range.head, + commit_range.span, + commit_range.complete, ) return Investigation( project=project, hg_revision=hg_revision, harness=_harness(tags), + debug_build=_is_debug_build(tags), failing_groups=groups, last_green_revision=last_green, - candidate_commits=candidate_commits, + commit_range=commit_range, ) diff --git a/agents/test-repair/tests/test_agent_loop.py b/agents/test-repair/tests/test_agent_loop.py index f37c06879f..a24eed131c 100644 --- a/agents/test-repair/tests/test_agent_loop.py +++ b/agents/test-repair/tests/test_agent_loop.py @@ -1,31 +1,75 @@ import asyncio import json +import subprocess from types import SimpleNamespace from hackbot_agents.test_repair import agent -from hackbot_agents.test_repair.resolve import FailingGroup, Investigation +from hackbot_agents.test_repair.resolve import ( + CommitRange, + FailingGroup, + Investigation, +) -def _result_msg(): +def _result_msg(is_error=False): return SimpleNamespace( - is_error=False, total_cost_usd=0.1, num_turns=3, result=None, subtype=None + is_error=is_error, + total_cost_usd=0.1, + num_turns=3, + result="max turns" if is_error else None, + subtype=None, ) -def _investigation(): +def _git_repo(path): + """A two-commit repo standing in for the pinned shallow checkout. + + The culprit is validated with ``git rev-parse``, so the tests need real + commits. Returns [base, head]. + """ + + def git(*args): + return subprocess.run( + ["git", "-C", str(path), *args], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + subprocess.run(["git", "init", "-q", str(path)], check=True, capture_output=True) + git("config", "user.email", "t@example.com") + git("config", "user.name", "T") + shas = [] + for name in ("base", "head"): + (path / f"{name}.txt").write_text(name) + git("add", "-A") + git("commit", "-qm", name) + shas.append(git("rev-parse", "HEAD")) + return shas + + +def _investigation(head, base, complete=True): return Investigation( project="autoland", hg_revision="hgrev", harness="mochitest", + debug_build=False, failing_groups=[ FailingGroup("dom/base/test/mochitest.ini", "dom/base/test/a.js") ], - last_green_revision="greensha", - candidate_commits=["headsha", "oldsha"], + last_green_revision="greenhg", + commit_range=CommitRange(head=head, base=base, span=2, complete=complete), ) -def _run(tmp_path, verdicts, monkeypatch): +def _fx_ctx(tmp_path): + return SimpleNamespace(mozconfig=tmp_path / ".mozconfig", objdir=tmp_path / "obj") + + +def _run(tmp_path, verdicts, monkeypatch, complete=True, results=None): + repo = tmp_path / "src" + repo.mkdir() + base, head = _git_repo(repo) scratch_out = tmp_path / "out" scratch_out.mkdir() calls = [] @@ -33,10 +77,13 @@ def _run(tmp_path, verdicts, monkeypatch): async def fake_session(reporter, options, prompt): calls.append(prompt) verdict = verdicts.pop(0) - (scratch_out / "verdict.json").write_text(json.dumps(verdict)) + if verdict is not None: + if verdict.get("culprit_commit") == "HEAD": + verdict = {**verdict, "culprit_commit": head} + (scratch_out / "verdict.json").write_text(json.dumps(verdict)) (scratch_out / "summary.md").write_text("the verdict") (scratch_out / "analysis.md").write_text("the reasoning") - return _result_msg() + return _result_msg(is_error=bool(results and results.pop(0))) monkeypatch.setattr(agent, "_run_session", fake_session) monkeypatch.setattr(agent, "build_sdk_server", lambda *a, **k: {"type": "sdk"}) @@ -44,30 +91,26 @@ async def fake_session(reporter, options, prompt): result = asyncio.run( agent.run_test_repair( bugzilla_mcp_server=None, - source_repo=tmp_path, - fx_ctx=object(), - investigation=_investigation(), + source_repo=repo, + fx_ctx=_fx_ctx(tmp_path), + investigation=_investigation(head, base if complete else None, complete), task_logs={}, scratch_out=scratch_out, verbose=False, log=None, ) ) - return result, calls + return result, calls, head def test_culprit_runs_fix_stage(tmp_path, monkeypatch): - result, calls = _run( + result, calls, head = _run( tmp_path, [ - { - "recommendation": "backout", - "culprit_commit": "headsha", - "confidence": 0.9, - }, + {"recommendation": "backout", "culprit_commit": "HEAD", "confidence": 0.9}, { "recommendation": "land_fix", - "culprit_commit": "headsha", + "culprit_commit": "HEAD", "confidence": 0.9, "proposed_patch": True, }, @@ -76,62 +119,123 @@ def test_culprit_runs_fix_stage(tmp_path, monkeypatch): ) assert len(calls) == 2 # analysis + fix assert result.classification == "regression" - assert result.culprit_commit == "headsha" + assert result.culprit_commit == head assert result.recommendation == "land_fix" assert result.proposed_patch is True - assert result.last_green_revision == "greensha" + assert result.last_green_revision == "greenhg" assert result.num_turns == 6 + # The fix stage can only verify anything if a mozconfig exists. + assert (tmp_path / ".mozconfig").exists() def test_no_culprit_skips_fix_stage(tmp_path, monkeypatch): - result, calls = _run( + result, calls, _head = _run( + tmp_path, + [{"recommendation": "backout", "culprit_commit": None, "confidence": 0.3}], + monkeypatch, + ) + assert len(calls) == 1 # no fix stage + assert result.culprit_commit is None + assert result.proposed_patch is False + assert result.recommendation == "do_not_backout" + + +def test_hallucinated_culprit_is_discarded(tmp_path, monkeypatch): + result, calls, _head = _run( + tmp_path, + [{"recommendation": "backout", "culprit_commit": "deadbeefdeadbeef"}], + monkeypatch, + ) + assert len(calls) == 1 + assert result.culprit_commit is None + assert result.recommendation == "do_not_backout" + + +def test_fix_stage_verdict_rewrite_keeps_analysis_culprit(tmp_path, monkeypatch): + # The fix stage may rewrite the whole file; the culprit must survive that. + result, _calls, head = _run( tmp_path, [ { "recommendation": "backout", - "culprit_commit": None, - "confidence": 0.3, + "culprit_commit": "HEAD", + "culprit_bug": 123, + "confidence": 0.9, }, + {"recommendation": "land_fix", "proposed_patch": True}, ], monkeypatch, ) - assert len(calls) == 1 # no fix stage - assert result.culprit_commit is None - assert result.proposed_patch is False + assert result.culprit_commit == head + assert result.culprit_bug == 123 + assert result.confidence == 0.9 + assert result.recommendation == "land_fix" + + +def test_failed_fix_stage_still_publishes_analysis(tmp_path, monkeypatch): + result, calls, head = _run( + tmp_path, + [ + {"recommendation": "backout", "culprit_commit": "HEAD", "confidence": 0.9}, + None, + ], + monkeypatch, + results=[False, True], + ) + assert len(calls) == 2 + assert result.culprit_commit == head + assert result.recommendation == "backout" + assert result.analysis == "the reasoning" -def test_assemble_defaults_on_missing_verdict(tmp_path): +def test_complete_range_prompt_gives_a_base_anchored_range(tmp_path, monkeypatch): + _result, calls, head = _run(tmp_path, [{"culprit_commit": None}], monkeypatch) + assert "culprit is one of them" in calls[0] + # The agent enumerates the range itself rather than being handed every sha. + assert "git log --oneline" in calls[0] + assert f"..{head}" in calls[0] + + +def test_incomplete_range_prompt_does_not_assert_the_culprit(tmp_path, monkeypatch): + _result, calls, _head = _run( + tmp_path, [{"culprit_commit": None}], monkeypatch, complete=False + ) + assert "may predate" in calls[0] + assert "culprit is one of them" not in calls[0] + # Without a last-green base the range falls back to the clone depth. + assert "HEAD~2..HEAD" in calls[0] + + +def test_assemble_defaults_on_empty_verdict(tmp_path): out = tmp_path / "out" out.mkdir() result = agent._assemble_result( out, + verdict={}, + source_repo=tmp_path, last_green_revision=None, total_turns=1, total_cost=0.0, publish_file=None, ) - # The agent assumes a regression; a missing verdict defaults accordingly. + # A regression is assumed, but there is no culprit to back out. assert result.classification == "regression" - assert result.recommendation == "backout" + assert result.recommendation == "do_not_backout" def test_assemble_tolerates_malformed_verdict_fields(tmp_path): - # verdict.json is authored by the model; bad confidence/bug must not crash the - # run after both stages have already done the expensive work. + # verdict.json is model-authored; bad fields must not crash a finished run. out = tmp_path / "out" out.mkdir() - (out / "verdict.json").write_text( - json.dumps( - { - "recommendation": "backout", - "culprit_commit": "abc", - "confidence": "high", - "culprit_bug": "n/a", - } - ) - ) result = agent._assemble_result( out, + verdict={ + "recommendation": "backout", + "culprit_commit": "abc", + "confidence": "high", + "culprit_bug": "n/a", + }, + source_repo=tmp_path, last_green_revision=None, total_turns=1, total_cost=0.0, @@ -140,10 +244,51 @@ def test_assemble_tolerates_malformed_verdict_fields(tmp_path): assert result.confidence == 0.0 assert result.culprit_bug is None assert result.classification == "regression" - assert result.culprit_commit == "abc" + # "abc" resolves to no commit in the checkout, so the blame is dropped. + assert result.culprit_commit is None + + +def test_assemble_reports_intermittent_classification(tmp_path): + out = tmp_path / "out" + out.mkdir() + result = agent._assemble_result( + out, + verdict={"classification": "intermittent", "intermittent_bug": 42}, + source_repo=tmp_path, + last_green_revision=None, + total_turns=1, + total_cost=0.0, + publish_file=None, + ) + assert result.classification == "intermittent" + assert result.intermittent_bug == 42 + assert result.recommendation == "do_not_backout" def test_coerce_recommendation_defaults_by_classification(): - assert agent._coerce_recommendation("bogus", "regression") == "backout" - assert agent._coerce_recommendation("bogus", "intermittent") == "do_not_backout" - assert agent._coerce_recommendation("land_fix", "regression") == "land_fix" + assert agent._coerce_recommendation("bogus", "regression", True) == "backout" + assert ( + agent._coerce_recommendation("bogus", "intermittent", False) == "do_not_backout" + ) + assert agent._coerce_recommendation("land_fix", "regression", True) == "land_fix" + # "backout" is meaningless without a commit to back out. + assert agent._coerce_recommendation("backout", "regression", False) == ( + "do_not_backout" + ) + + +def test_resolve_culprit_normalizes_against_the_checkout(tmp_path): + repo = tmp_path / "src" + repo.mkdir() + _base, head = _git_repo(repo) + assert agent._resolve_culprit(repo, head) == head + assert agent._resolve_culprit(repo, head[:8]) == head + assert agent._resolve_culprit(repo, "deadbeefdeadbeefdeadbeef") is None + assert agent._resolve_culprit(repo, None) is None + assert agent._resolve_culprit(repo, " ") is None + + +def test_range_expr_prefers_the_last_green_base(): + anchored = CommitRange(head="h" * 40, base="b" * 40, span=5, complete=True) + assert agent._range_expr(anchored) == f"{'b' * 40}..{'h' * 40}" + assert agent._range_expr(CommitRange("h" * 40, None, 5, False)) == "HEAD~5..HEAD" diff --git a/agents/test-repair/tests/test_resolve.py b/agents/test-repair/tests/test_resolve.py index f40ea99b93..f1523007e4 100644 --- a/agents/test-repair/tests/test_resolve.py +++ b/agents/test-repair/tests/test_resolve.py @@ -29,10 +29,20 @@ def test_harness_detection(): assert resolve._harness({"test-suite": "xpcshell"}) == "xpcshell" assert resolve._harness({"label": "test-linux/opt-xpcshell-4"}) == "xpcshell" assert resolve._harness({"test-suite": "mochitest-browser-chrome"}) == "mochitest" - assert resolve._harness({"kind": "web-platform-tests"}) == "web-platform-tests" + # Every Firefox test task has kind=="test", so the suite must win over it. + assert ( + resolve._harness({"kind": "test", "test-suite": "web-platform-tests"}) + == "web-platform-tests" + ) assert resolve._harness({}) == "unknown" +def test_debug_build_detection(): + assert resolve._is_debug_build({"test-platform": "linux1804-64-qr/debug"}) is True + assert resolve._is_debug_build({"test-platform": "windows11-64/opt"}) is False + assert resolve._is_debug_build({}) is False + + def test_last_green_returns_first_passing_ancestor(monkeypatch): green = FakePush("greenrev", {GROUP: FakeSummary(Status.PASS)}) flaky = FakePush( @@ -58,41 +68,84 @@ def boom(rev, branch=None): assert resolve._last_green("autoland", "headrev", GROUP) is None -def test_candidate_commits_range_head_first(monkeypatch): - pushes = { - "1": {"changesets": [{"node": "hgA"}], "git_changesets": ["gitA"]}, - "2": { - "changesets": [{"node": "hgB"}, {"node": "hgC"}], - "git_changesets": ["gitB", "gitC"], - }, - } +def _hg2git(rev): + return {"hgA": "gitA", "hgB": "gitB", "hgC": "gitC"}.get(rev) + + +def test_resolve_range_maps_only_the_endpoints(monkeypatch): + pushes = {"2": {"changesets": [{"node": "hgB"}, {"node": "hgC"}]}} + looked_up = [] + + def fake_hg_to_git(rev): + looked_up.append(rev) + return _hg2git(rev) + monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) - commits = resolve._candidate_commits("autoland", "hgC", "hgA", 50) - assert commits == ["gitC", "gitB", "gitA"] + monkeypatch.setattr(resolve, "_hg_to_git", fake_hg_to_git) + rng = resolve._resolve_range("autoland", "hgC", "hgA", 100) + assert (rng.head, rng.base, rng.span, rng.complete) == ("gitC", "gitA", 2, True) + # Two lando lookups regardless of range width; no per-commit mapping to fail. + assert looked_up == ["hgC", "hgA"] -def test_candidate_commits_capped_newest_first(monkeypatch): - pushes = { - str(i): {"changesets": [{"node": f"hg{i}"}], "git_changesets": [f"git{i}"]} - for i in range(1, 4) - } +def test_resolve_range_without_last_green_is_incomplete(monkeypatch): + pushes = {"1": {"changesets": [{"node": "hgA"}]}} + monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) + monkeypatch.setattr(resolve, "_hg_to_git", _hg2git) + rng = resolve._resolve_range("autoland", "hgA", None, 100) + assert rng.head == "gitA" + assert rng.base is None + assert rng.complete is False + + +def test_resolve_range_capped_drops_the_base(monkeypatch): + pushes = {str(i): {"changesets": [{"node": f"hg{i}"}]} for i in range(1, 6)} monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) - commits = resolve._candidate_commits("autoland", "hg3", "hg0", 2) - assert commits == ["git3", "git2"] + monkeypatch.setattr(resolve, "_hg_to_git", lambda rev: "gitC") + rng = resolve._resolve_range("autoland", "hgC", "hgA", 2) + # The base falls outside the capped clone, so it can no longer anchor it. + assert rng.base is None + assert rng.span == 2 + assert rng.complete is False + +def test_resolve_range_none_when_head_unresolvable(monkeypatch): + monkeypatch.setattr(resolve, "_hg_to_git", lambda rev: None) + assert resolve._resolve_range("autoland", "hgHEAD", "hgOLD", 100) is None -def test_candidate_commits_falls_back_to_head_commit(monkeypatch): + +def test_resolve_range_incomplete_when_base_unresolvable(monkeypatch): + pushes = {"1": {"changesets": [{"node": "hgB"}]}} + monkeypatch.setattr(resolve, "_get_json", lambda url: {"pushes": pushes}) + monkeypatch.setattr( + resolve, "_hg_to_git", lambda rev: "gitB" if rev == "hgB" else None + ) + rng = resolve._resolve_range("autoland", "hgB", "hgA", 100) + assert rng.head == "gitB" + assert rng.base is None + assert rng.complete is False + + +def test_resolve_range_survives_pushlog_failure(monkeypatch): def boom(url): raise resolve.requests.exceptions.RequestException("hg down") monkeypatch.setattr(resolve, "_get_json", boom) monkeypatch.setattr(resolve, "_hg_to_git", lambda rev: "gitHEAD") - assert resolve._candidate_commits("autoland", "hgHEAD", "hgOLD", 50) == ["gitHEAD"] + rng = resolve._resolve_range("autoland", "hgHEAD", "hgOLD", 100) + assert rng.head == "gitHEAD" + assert rng.base is None + assert rng.span == 1 + assert rng.complete is False def test_resolve_investigation_assembles_context(monkeypatch): task = { - "tags": {"project": "autoland", "test-suite": "mochitest-browser-chrome"}, + "tags": { + "project": "autoland", + "test-suite": "mochitest-browser-chrome", + "test-platform": "linux1804-64-qr/debug", + }, "payload": {"env": {"GECKO_HEAD_REV": "hghead"}}, } monkeypatch.setattr(resolve, "_get_json", lambda url: task) @@ -100,15 +153,34 @@ def test_resolve_investigation_assembles_context(monkeypatch): resolve, "_failing_groups", lambda tid: [FailingGroup(GROUP, "a.js")] ) monkeypatch.setattr(resolve, "_last_green", lambda *a: "greenrev") - monkeypatch.setattr(resolve, "_candidate_commits", lambda *a: ["gitHead", "gitOld"]) + monkeypatch.setattr( + resolve, + "_resolve_range", + lambda *a: resolve.CommitRange("gitHead", "gitBase", 2, True), + ) inv = resolve.resolve_investigation("TASK") assert inv.project == "autoland" assert inv.hg_revision == "hghead" assert inv.harness == "mochitest" + assert inv.debug_build is True assert inv.last_green_revision == "greenrev" - assert inv.candidate_commits == ["gitHead", "gitOld"] assert inv.failure_commit == "gitHead" + assert inv.commit_range.base == "gitBase" + assert inv.commit_range.complete is True + assert inv.commit_range.span == 2 + + +def test_resolve_investigation_requires_a_git_commit(monkeypatch): + task = { + "tags": {"project": "autoland"}, + "payload": {"env": {"GECKO_HEAD_REV": "hghead"}}, + } + monkeypatch.setattr(resolve, "_get_json", lambda url: task) + monkeypatch.setattr(resolve, "_failing_groups", lambda tid: []) + monkeypatch.setattr(resolve, "_resolve_range", lambda *a: None) + with pytest.raises(ValueError): + resolve.resolve_investigation("TASK") def test_resolve_investigation_requires_revision(monkeypatch): diff --git a/agents/test-repair/tests/test_scaffold.py b/agents/test-repair/tests/test_scaffold.py index f7d0248df4..6b79df0743 100644 --- a/agents/test-repair/tests/test_scaffold.py +++ b/agents/test-repair/tests/test_scaffold.py @@ -3,6 +3,7 @@ from hackbot_agents.test_repair import logs from hackbot_agents.test_repair.__main__ import _pin_checkout from hackbot_agents.test_repair.agent import TestRepairResult +from hackbot_agents.test_repair.resolve import CommitRange, Investigation def test_sanitize_log_keeps_failure_and_error_lines(): @@ -30,12 +31,25 @@ def test_sanitize_log_dedupes_consecutive_repeats(): assert len(logs.sanitize_log(raw).splitlines()) == 1 +def _investigation(**kwargs): + defaults = dict( + project="autoland", + hg_revision="hgrev", + harness="mochitest", + debug_build=False, + failing_groups=[], + last_green_revision="greenhg", + commit_range=CommitRange(head="headsha", base="basesha", span=3, complete=True), + ) + return Investigation(**{**defaults, **kwargs}) + + def test_pin_checkout_sets_ref_and_depth(monkeypatch): monkeypatch.delenv("SOURCE_REF", raising=False) monkeypatch.delenv("SOURCE_DEPTH", raising=False) - _pin_checkout(["headsha", "midsha", "oldsha"]) + _pin_checkout(_investigation()) assert os.environ["SOURCE_REF"] == "headsha" - # Depth spans the 3 candidates plus one so the last-green parent is reachable. + # Depth spans the 3 commits in the range plus the base's parent. assert os.environ["SOURCE_DEPTH"] == "4" diff --git a/services/hackbot-ui/components/TriggerForm.tsx b/services/hackbot-ui/components/TriggerForm.tsx index 8c2dc6e5f7..abadee5a58 100644 --- a/services/hackbot-ui/components/TriggerForm.tsx +++ b/services/hackbot-ui/components/TriggerForm.tsx @@ -47,7 +47,9 @@ export function TriggerForm() { const isReproAgent = agent === "autowebcompat-repro"; const isBuildRepairAgent = agent === "build-repair"; + const isTestRepairAgent = agent === "test-repair"; const isTestPlanAgent = agent === "test-plan-generator"; + const needsFailureTasks = isBuildRepairAgent || isTestRepairAgent; async function onSubmit(e: React.FormEvent) { e.preventDefault(); @@ -59,13 +61,15 @@ export function TriggerForm() { const hasBugId = Number.isInteger(parsedBugId) && parsedBugId > 0; const hasBugData = isReproAgent && bugData.trim().length > 0; - if (isBuildRepairAgent) { - if (hasBugId) inputs.bug_id = parsedBugId; - if (!gitCommit.trim()) { - setError("Enter a git commit hash."); - return; + if (needsFailureTasks) { + if (isBuildRepairAgent) { + if (hasBugId) inputs.bug_id = parsedBugId; + if (!gitCommit.trim()) { + setError("Enter a git commit hash."); + return; + } + inputs.git_commit = gitCommit.trim(); } - inputs.git_commit = gitCommit.trim(); if (!failureTasks.trim()) { setError("Enter failure tasks as a JSON object."); return; @@ -90,7 +94,7 @@ export function TriggerForm() { return; } inputs.failure_tasks = parsedTasks; - inputs.run_try_push = runTryPush; + if (isBuildRepairAgent) inputs.run_try_push = runTryPush; } else if (isTestPlanAgent) { if (!featureName.trim()) { setError("Enter a feature name."); @@ -127,7 +131,7 @@ export function TriggerForm() { const n = Number.parseInt(maxTurns, 10); if (Number.isInteger(n) && n > 0) inputs.max_turns = n; } - if (!isBuildRepairAgent && effort.trim()) inputs.effort = effort.trim(); + if (!needsFailureTasks && effort.trim()) inputs.effort = effort.trim(); setSubmitting(true); try { @@ -143,11 +147,14 @@ export function TriggerForm() { const run = body as RunRef; const label = isBuildRepairAgent ? `commit ${gitCommit.trim().slice(0, 12)}` - : isTestPlanAgent - ? featureName.trim() - : hasBugId - ? `bug ${parsedBugId}` - : "inline report"; + : isTestRepairAgent + ? Object.keys(inputs.failure_tasks as Record)[0] ?? + "test failure" + : isTestPlanAgent + ? featureName.trim() + : hasBugId + ? `bug ${parsedBugId}` + : "inline report"; saveRun({ run_id: run.run_id, agent: run.agent, @@ -184,7 +191,7 @@ export function TriggerForm() { - {!isBuildRepairAgent && !isTestPlanAgent && ( + {!needsFailureTasks && !isTestPlanAgent && (
+ + )} -
- -