Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions agents/build-repair/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ It also optionally bootstraps Firefox build if needed.
reaches `CHECKOUT_DEPTH` commits back so the agent can find a culprit in an earlier
push when the failing job did not run there.
- `GIT_COMMIT` - Optional override for the failure commit (skips the hg->git lookup).
- `BUG_ID` - Optional Bugzilla bug id.
- `BUG_ID` - Optional override, normally unset. The bug is resolved from the push: the
pushlog lookup already returns each changeset's description, whose first line names it
(`Bug 123 - ...`). The failure commit's bug gives the analysis stage its Bugzilla
context; the fix is filed against the _blamed_ commit's bug, known once stage 1 picks
the culprit.

## Output

Expand All @@ -38,11 +42,29 @@ Second stage - fixing:

The result reports `blamed_commit` so the caller can attribute the failure to a developer.

## Submitting the fix

Once a bug is known, the fix stage records a `phabricator.submit_patch` action in
`summary.json` -- a new WIP revision carrying the fix, whose diff the runtime builds
from the agent's own checkout into `changes/phabricator_diff.json`. Nothing is posted
to the bug, and nothing reaches Phabricator during the run.

A developer reviews the patch (`changes/changes.patch`) in the Hackbot UI and applies
the action from there; the full review then happens on the revision. Unlike the email
below, this one waits for a human -- only `email.send` auto-applies, see
[`agents.py`](../../services/hackbot-api/app/agents.py). Eval runs pass no actions
recorder, so they never record it.

A run whose blamed commit names no bug (a "No bug" commit, a backout) produces the fix
but records no revision -- one has to be filed against a bug. The agent log says so
when that happens.

## Email notification

A run that produced a patch records an `email.send` action carrying the analysis, the
blamed commit and the patch, addressed to that commit's author and to the developer who
pushed the failing change (the hackbot team is copied apply-side). A run that proposed no
blamed commit, the patch and -- when a revision is pending -- the steps to review and
apply it, addressed to that commit's author and to the developer who pushed the failing
change (the hackbot team is copied apply-side). A run that proposed no
patch is a transient or not-to-blame failure and is not emailed -- see
`NOTIFY_ONLY_WITH_PATCH` in [config.py](hackbot_agents/build_repair/config.py).

Expand Down
7 changes: 7 additions & 0 deletions agents/build-repair/hackbot_agents/build_repair/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from hackbot_runtime import HackbotContext, run_async
from hackbot_runtime.actions.email import record_email
from hackbot_runtime.actions.phabricator import PATCH_ACTION_TYPES
from pydantic_settings import BaseSettings, SettingsConfigDict

from .agent import BuildRepairResult, run_build_repair
Expand Down Expand Up @@ -56,6 +57,7 @@ async def main(ctx: HackbotContext) -> BuildRepairResult:
source_repo=ctx.repo_path,
fx_ctx=ctx.firefox,
bug_id=inputs.bug_id,
commit_bugs=push.commit_bugs,
git_commits=git_commits,
project=push.project,
hg_revision=push.hg_revision,
Expand All @@ -66,6 +68,7 @@ async def main(ctx: HackbotContext) -> BuildRepairResult:
log=ctx.log_path,
verbose=True,
publish_file=ctx.publish_file,
actions_recorder=ctx.actions,
)

try:
Expand All @@ -85,12 +88,16 @@ def _record_analysis_email(
return

blamed_author = resolve_author_email(ctx.repo_path, result.blamed_commit)
revision_pending = any(
action["type"] in PATCH_ACTION_TYPES for action in ctx.actions.actions
)
subject, body = build_email(
result,
push,
task_id=task_id,
run_id=ctx.run_id,
has_patch=has_patch,
revision_pending=revision_pending,
blamed_author=blamed_author,
)
record_email(
Expand Down
127 changes: 107 additions & 20 deletions agents/build-repair/hackbot_agents/build_repair/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import json
import re
import subprocess
import sys
import tempfile
from collections.abc import Callable
Expand All @@ -35,7 +36,9 @@
UserMessage,
)
from hackbot_agents.build_repair.try_push import TRY_TOOLS
from hackbot_runtime import AgentError, HackbotAgentResult
from hackbot_runtime import ActionsRecorder, AgentError, HackbotAgentResult
from hackbot_runtime.actions import ACTIONS_SERVER_NAME
from hackbot_runtime.actions.claude_sdk import actions_server_for, actions_to_tool_names
from hackbot_runtime.claude import Reporter

from .config import (
Expand All @@ -45,6 +48,7 @@
BUGZILLA_READ_TOOLS,
BUILD_TOOL,
CHECKOUT_DEPTH,
ENABLED_ACTION_TYPES,
FIREFOX_TOOLS,
FIX_MODEL,
TRY_PUSH_TOOL,
Expand All @@ -58,12 +62,13 @@
FIX_TEMPLATE,
PUSH_COMMIT_LINE,
PUSH_CONTEXT,
REPORT_INSTRUCTIONS,
SINGLE_COMMIT_CONTEXT,
TREEHERDER_STEP,
TREEHERDER_STEP_NO_PUSH,
TRY_PUSH_INSTRUCTIONS,
)
from .resolve import task_push
from .resolve import _bug_from_desc, task_push

TARGET_SOFTWARE = "Mozilla Firefox"

Expand Down Expand Up @@ -148,6 +153,7 @@ async def run_build_repair(
source_repo: Path,
fx_ctx: FirefoxContext,
bug_id: int | None = None,
commit_bugs: dict[str, int] | None = None,
git_commits: list[str],
project: str | None = None,
hg_revision: str | None = None,
Expand All @@ -158,19 +164,40 @@ async def run_build_repair(
verbose: bool = False,
log: Path | None = None,
publish_file: Callable[[str, Path, str | None], str] | None = None,
actions_recorder: ActionsRecorder | None = None,
) -> BuildRepairResult:
"""Analyze a build failure and implement a fix in ``source_repo``.

Returns a :class:`BuildRepairResult`; raises :class:`AgentError` if a stage
ends in an error or produces no result.

``commit_bugs`` maps each push commit to the bug it landed for (from the
pushlog, see ``resolve_push``). A run is never told which bug a failure
belongs to, so that mapping is what an unset ``bug_id`` falls back to.

Pass ``actions_recorder`` to let the fix stage submit the fix for review as a
Phabricator revision, recorded as a proposed action rather than submitted
here. Runs without a recorder, or with no bug for the blamed commit, only
produce the fix in the source tree.
"""
if not git_commits:
raise AgentError("git_commits must contain at least one commit")
failure_commit = git_commits[0]
label = f"bug {bug_id}" if bug_id is not None else f"commit {failure_commit[:12]}"
commit_bugs = commit_bugs or {}
# The failure commit's bug gives stage 1 its Bugzilla context. Which bug the
# *fix* is filed against is only settled once stage 1 picks the culprit out
# of the push, so it is resolved again below.
push_bug_id = bug_id if bug_id is not None else commit_bugs.get(failure_commit)
label = (
f"bug {push_bug_id}"
if push_bug_id is not None
else f"commit {failure_commit[:12]}"
)
print(f"[build_repair] repairing {label} at {failure_commit}", file=sys.stderr)

scratch_dir = Path(tempfile.mkdtemp(prefix=f"build-repair-{bug_id or 'nobug'}-"))
scratch_dir = Path(
tempfile.mkdtemp(prefix=f"build-repair-{push_bug_id or 'nobug'}-")
)
scratch_out = scratch_dir / "out"
scratch_out.mkdir(parents=True, exist_ok=True)

Expand Down Expand Up @@ -213,19 +240,16 @@ async def run_build_repair(
treeherder_step=treeherder_step,
blame_step=_blame_step(git_commits, scratch_out),
scratch_out=scratch_out,
bug_context=BUG_CONTEXT.format(bug_id=bug_id) if bug_id is not None else "",
bug_step=BUG_ANALYSIS_STEP.format(bug_id=bug_id) if bug_id is not None else "",
logs_num=3 if bug_id is not None else 2,
)
fix_prompt = FIX_TEMPLATE.format(
target_software=TARGET_SOFTWARE,
source_repo=source_repo,
scratch_out=scratch_out,
try_push=(
TRY_PUSH_INSTRUCTIONS.format(task_name=task_name) if run_try_push else ""
bug_context=(
BUG_CONTEXT.format(bug_id=push_bug_id) if push_bug_id is not None else ""
),
bug_step=(
BUG_ANALYSIS_STEP.format(bug_id=push_bug_id)
if push_bug_id is not None
else ""
),
logs_num=3 if push_bug_id is not None else 2,
)

total_cost = 0.0
total_turns = 0
# Last JSON result of each tracked tool, keyed by tool name. Lets us report
Expand Down Expand Up @@ -253,17 +277,67 @@ async def run_build_repair(
total_cost += result_msg.total_cost_usd or 0.0
total_turns += result_msg.num_turns or 0

# Which bug the fix belongs to is only settled now: when the push has
# several commits, stage 1 is what picks the culprit, and it is the
# culprit's bug a revision has to be filed against.
blamed_commit = _resolve_blame(scratch_out, git_commits)
resolved_bug_id = bug_id
if resolved_bug_id is None and blamed_commit is not None:
# A culprit from an earlier push is not in the pushlog mapping, but
# the checkout reaches CHECKOUT_DEPTH commits back, so its subject
# still names the bug.
resolved_bug_id = commit_bugs.get(blamed_commit) or _bug_from_commit(
source_repo, blamed_commit
)

# Reporting is confined to the fix stage: the analysis stage must not
# submit anything before there is a verified fix.
report = actions_recorder is not None and resolved_bug_id is not None
fix_mcp_servers = mcp_servers
fix_allowed_tools = allowed_tools
if report:
_, actions_server = actions_server_for(
actions_recorder, types=ENABLED_ACTION_TYPES
)
fix_mcp_servers = {**mcp_servers, ACTIONS_SERVER_NAME: actions_server}
fix_allowed_tools = [
*allowed_tools,
*actions_to_tool_names(ENABLED_ACTION_TYPES),
]
elif actions_recorder is not None:
print(
f"[build_repair] no bug for blamed commit {blamed_commit}: the fix "
"will be produced but not submitted for review, since a "
"Phabricator revision is filed against a bug",
file=sys.stderr,
)

fix_prompt = FIX_TEMPLATE.format(
target_software=TARGET_SOFTWARE,
source_repo=source_repo,
scratch_out=scratch_out,
try_push=(
TRY_PUSH_INSTRUCTIONS.format(task_name=task_name)
if run_try_push
else ""
),
report=(
REPORT_INSTRUCTIONS.format(bug_id=resolved_bug_id) if report else ""
),
)

# Stage 2: fix (lower effort, edits the source tree and verifies it
# builds against a mozconfig that mirrors the failing CI config).
_write_mozconfig(fx_ctx)
reporter.header(f"{label}: fix")
fix_label = f"bug {resolved_bug_id}" if resolved_bug_id is not None else label
reporter.header(f"{fix_label}: fix")
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,
mcp_servers=fix_mcp_servers,
allowed_tools=fix_allowed_tools,
max_turns=max_turns,
)
result_msg = await _run_session(
Expand All @@ -278,10 +352,9 @@ async def run_build_repair(

build_result = captured.get(BUILD_TOOL)
try_result = captured.get(TRY_PUSH_TOOL, {})
blamed_commit = _resolve_blame(scratch_out, git_commits)

return BuildRepairResult(
bug_id=bug_id,
bug_id=resolved_bug_id,
git_commit=failure_commit,
summary=summary,
analysis=analysis,
Expand Down Expand Up @@ -402,6 +475,20 @@ def _blame_step(git_commits: list[str], scratch_out: Path) -> str:
return BLAME_STEP.format(scratch_out=scratch_out)


def _bug_from_commit(repo: Path, sha: str) -> int | None:
"""The bug a commit landed for, read off its subject in the local checkout."""
try:
subject = subprocess.run(
["git", "-C", str(repo), "log", "-1", "--format=%s", sha],
check=True,
capture_output=True,
text=True,
).stdout
except (subprocess.CalledProcessError, OSError):
return None
return _bug_from_desc(subject)


_SHA_RE = re.compile(r"[0-9a-f]{7,40}$")


Expand Down
7 changes: 7 additions & 0 deletions agents/build-repair/hackbot_agents/build_repair/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
# Optional try-server tool, wired only when run_try_push is enabled.
TRY_PUSH_TOOL = "mcp__firefox__submit_try_push"

# Recordable action types the agent may take, by dotted id. Submitting the fix
# for review is the only one: the developer reviews the patch in the Hackbot UI
# and then in Phabricator, so the agent posts nothing to the bug itself. Needs a
# bug to file the revision against, so it is wired only when the run has a bug id
# (see run_build_repair).
ENABLED_ACTION_TYPES = ["phabricator.submit_patch"]

# The agent always runs inside an isolated Docker container, so there is no
# sandbox and tools run without per-command permission prompts (see
# permission_mode="bypassPermissions" in agent.py). This is just the set of
Expand Down
17 changes: 16 additions & 1 deletion agents/build-repair/hackbot_agents/build_repair/notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,14 @@ def build_email(
task_id: str,
run_id: str,
has_patch: bool = False,
revision_pending: bool = False,
blamed_author: str | None = None,
) -> tuple[str, str]:
"""The subject and markdown body of the build-failure email."""
"""The subject and markdown body of the build-failure email.

``revision_pending`` means the run recorded a ``phabricator.submit_patch``
action that is waiting for approval, so the email says how to apply it.
"""
failure_commit = push.git_commits[0]
subject = (
f"[build-repair] Build failure analysis for "
Expand Down Expand Up @@ -163,6 +168,16 @@ def build_email(
"",
f"- Local build verified: {result.local_build_verified}",
]
if revision_pending:
lines += [
"",
"## How to submit the fix to Phabricator",
"",
"1. Check the patch on the run page: " + RUN_URL.format(run_id=run_id),
"2. Press *Apply pending actions* there to open a WIP revision for "
+ _link(BUG_URL.format(bug_id=result.bug_id), f"bug {result.bug_id}")
+ ", where the revision appears; review and land it as usual.",
]
if has_patch:
# The diff itself is substituted for the placeholder when the mail is sent,
# from the same artifact it attaches.
Expand Down
10 changes: 9 additions & 1 deletion agents/build-repair/hackbot_agents/build_repair/prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@
missing toolchain (e.g. rustc or clang), run the bootstrap_firefox tool once and
then build again. Verify via the build_firefox tool rather than a raw `./mach
build` so the build result is recorded.
{try_push}
{try_push}{report}

Do not prompt to edit files. Work fully autonomously, do not ask any questions.
Use all allowed tools without prompting.
Expand All @@ -173,3 +173,11 @@
Once the fix builds locally, validate it on CI: call the submit_try_push tool with the
failing task name ('{task_name}') to push to the try server and report the build result.
"""

REPORT_INSTRUCTIONS = """
Once the build is verified, submit the fix with the `phabricator_submit_patch`
action: bug_id={bug_id}, a title of the form "Bug {bug_id} - <what the fix does>",
and a summary naming the busted commit, the failing task and the root cause.
If the fix does not build, or you are not confident in it, record nothing and say
so in your final message.
"""
Comment thread
evgenyrp marked this conversation as resolved.
Loading