Improve email for build repair

services/hackbot-pulse-listener/README.md

@@ -2,8 +2,9 @@
 
 Listens to Taskcluster build-failure pulse messages, and for failed **Firefox build
 tasks** triggers the `build-repair` hackbot agent through the hackbot-api. When a run
-finishes (minutes later) it emails the developer who pushed the change a link to the
-hackbot UI and a summary of the analysis and fix.
+finishes (minutes later) it emails a link to the hackbot UI, a summary of the analysis
+and fix, a Treeherder link to the failing job, and the commit the agent blamed for the
+failure. The email's primary recipient is the author of that blamed commit.
 
 ## How it works
 
@@ -14,8 +15,11 @@ hackbot UI and a summary of the analysis and fix.
 3. Fetch the task definition to read `GECKO_HEAD_REV` (the revision is not in the message).
 4. Dedupe by revision with an in-memory TTL cache, so only one agent run is triggered per
    revision even when many build tasks fail for the same push.
-5. `POST /agents/build-repair/runs`, then poll `GET /runs/{run_id}` until terminal and send
-   the report email.
+5. Trigger the agent with the failing tasks -- it resolves the push commits itself and
+   returns the commit it blamed for the failure, so the listener does no pushlog lookups.
+6. `POST /agents/build-repair/runs`, then poll `GET /runs/{run_id}` until terminal. Look the
+   blamed commit up in the firefox GitHub mirror to get its author's email and send the
+   report email to that developer.
 
 The dedupe cache and pending-run tracking are in-memory (reset on restart).
 
@@ -31,9 +35,14 @@ uv run --package hackbot-pulse-listener python -m app
 ```
 
 Email is sent only when `SENDGRID_API_KEY` and `NOTIFICATION_SENDER` are set; otherwise it
-is logged and skipped. Notifications go to the revision author and, if set, the `NOTIFICATION_TEAM_EMAIL` team address. Set `NOTIFICATION_OVERRIDE_EMAIL` to route every notification to a
+is logged and skipped. The blamed commit's author (looked up in the firefox GitHub mirror),
+the pushing developer, and, if set, the
+`NOTIFICATION_TEAM_EMAIL` team address all get the email, which spells out why each
+recipient is on it. Set `NOTIFICATION_OVERRIDE_EMAIL` to route every notification to a
 single address (useful for local testing). By default only runs that produced a patch are
 emailed; set `NOTIFY_ONLY_WITH_PATCH=false` to also notify on transient / not-to-blame runs.
+When `NOTIFICATION_TEAM_EMAIL` is set, notifications use it as `Reply-To` so recipients can
+reply with feedback on the analysis.
 
 ## Test
 

services/hackbot-pulse-listener/app/config.py

@@ -17,6 +17,7 @@ class Settings(BaseSettings):
     firefox_git_url: str = "https://github.com/mozilla-firefox/firefox"
     firefox_hg_url: str = "https://hg.mozilla.org/mozilla-unified"
     bugzilla_url: str = "https://bugzilla.mozilla.org"
+    treeherder_url: str = "https://treeherder.mozilla.org"
 
     # Failure filtering and agent inputs.
     # ``watched_repos`` is a comma-separated list of Taskcluster ``project`` tags.

services/hackbot-pulse-listener/app/consumer.py

@@ -82,7 +82,6 @@ def process(body: dict, executor: Executor) -> str | None:
         return None
 
     inputs: dict = {
-        "git_commit": git_commit,
         "failure_tasks": {task_name: task_id},
         "run_try_push": settings.run_try_push,
     }

services/hackbot-pulse-listener/app/github.py

@@ -0,0 +1,32 @@
+import logging
+
+import httpx
+
+from app.config import settings
+
+logger = logging.getLogger(__name__)
+
+_TIMEOUT = httpx.Timeout(30.0)
+
+
+def _repo_slug() -> str:
+    """``owner/repo`` parsed from the configured firefox git url."""
+    return settings.firefox_git_url.rstrip("/").removeprefix("https://github.com/")
+
+
+def commit_author_email(git_commit: str) -> str | None:
+    """Author email of a firefox git commit, or None.
+
+    The build-repair agent returns the commit it blamed for the failure; we look
+    that commit up in the firefox GitHub mirror to notify its author directly.
+    """
+    url = f"https://api.github.com/repos/{_repo_slug()}/commits/{git_commit}"
+    headers = {"Accept": "application/vnd.github+json"}
+    try:
+        resp = httpx.get(url, headers=headers, timeout=_TIMEOUT)
+        resp.raise_for_status()
+        author = (resp.json().get("commit") or {}).get("author") or {}
+    except (httpx.HTTPError, ValueError) as exc:
+        logger.warning("Failed to fetch author for commit %s: %s", git_commit, exc)
+        return None
+    return author.get("email") or None

services/hackbot-pulse-listener/app/notify.py

@@ -2,7 +2,7 @@
 import logging
 import re
 
-from app import client
+from app import client, github
 from app.config import settings
 from app.models import RunContext
 
@@ -26,7 +26,10 @@ def send_email(ctx: RunContext, run_doc: dict) -> None:
         logger.info("Run %s produced no patch; skipping notification", ctx.run_id)
         return
 
-    recipients = _recipients(ctx.developer_email)
+    findings = (run_doc.get("summary") or {}).get("findings") or {}
+    blamed_commit = findings.get("blamed_commit")
+    blamed_author = github.commit_author_email(blamed_commit) if blamed_commit else None
+    recipients = _recipients(blamed_author, ctx.developer_email)
     if not recipients:
         logger.info("No recipients for run %s; skipping notification", ctx.run_id)
         return
@@ -47,6 +50,7 @@ def send_email(ctx: RunContext, run_doc: dict) -> None:
         From,
         HtmlContent,
         Mail,
+        ReplyTo,
         Subject,
         To,
     )
@@ -55,7 +59,7 @@ def send_email(ctx: RunContext, run_doc: dict) -> None:
         f"[build-repair] Build failure analysis for {ctx.repo}@{ctx.git_commit[:12]}"
     )
 
-    body_md = _build_body(ctx, run_doc, patch)
+    body_md = _build_body(ctx, run_doc, patch, blamed_author)
     html = markdown2.markdown(body_md, extras=["fenced-code-blocks", "tables"])
 
     sg = sendgrid.SendGridAPIClient(api_key=settings.sendgrid_api_key)
@@ -74,6 +78,8 @@ def send_email(ctx: RunContext, run_doc: dict) -> None:
             FileType("text/x-patch"),
             Disposition("attachment"),
         )
+    if settings.notification_team_email:
+        message.reply_to = ReplyTo(settings.notification_team_email)
     response = sg.send(message=message)
     logger.info(
         "Sent build-repair notification to %s (status %s)",
@@ -82,16 +88,20 @@ def send_email(ctx: RunContext, run_doc: dict) -> None:
     )
 
 
-def _recipients(developer_email: str | None) -> list[str]:
-    """Recipients for a run: the revision author plus the team address, deduped.
+def _recipients(
+    author_email: str | None, developer_email: str | None = None
+) -> list[str]:
+    """Recipients for a run, deduped and ordered by priority.
 
+    The blamed commit's author is the primary To (the person who introduced the
+    failure), then the pushing developer, then the team address.
     ``notification_override_email`` short-circuits to a single address so local
     testing never mails real developers or the team.
     """
     if settings.notification_override_email:
         return [settings.notification_override_email]
     recipients: list[str] = []
-    for addr in (developer_email, settings.notification_team_email):
+    for addr in (author_email, developer_email, settings.notification_team_email):
         if addr and addr not in recipients:
             recipients.append(addr)
     return recipients
@@ -121,13 +131,26 @@ def _task_url(task_id: str) -> str:
     return f"{settings.taskcluster_root_url.rstrip('/')}/tasks/{task_id}"
 
 
+def _treeherder_url(repo: str, hg_revision: str, task_id: str) -> str:
+    return (
+        f"{settings.treeherder_url.rstrip('/')}/#/jobs"
+        f"?repo={repo}&revision={hg_revision}&selectedTaskRun={task_id}"
+    )
+
+
 def _bug_url(bug_id: object) -> str:
     return f"{settings.bugzilla_url.rstrip('/')}/show_bug.cgi?id={bug_id}"
 
 
-def _build_body(ctx: RunContext, run_doc: dict, patch: str | None = None) -> str:
+def _build_body(
+    ctx: RunContext,
+    run_doc: dict,
+    patch: str | None = None,
+    blamed_author: str | None = None,
+) -> str:
     summary = run_doc.get("summary") or {}
     findings = summary.get("findings") or {}
+    blamed_commit = findings.get("blamed_commit")
 
     lines = [
         "# Build failure analysis",
@@ -136,8 +159,17 @@ def _build_body(ctx: RunContext, run_doc: dict, patch: str | None = None) -> str
         f"- **Revision (git):** [`{ctx.git_commit[:12]}`]({_git_url(ctx.git_commit)})",
         f"- **Revision (hg):** [`{ctx.hg_revision[:12]}`]({_hg_url(ctx.hg_revision)})",
         f"- **Failed task:** [`{ctx.task_id}`]({_task_url(ctx.task_id)})",
+        f"- **Treeherder:** "
+        f"[jobs]({_treeherder_url(ctx.repo, ctx.hg_revision, ctx.task_id)})",
     ]
 
+    if blamed_commit:
+        by = f" by {blamed_author}" if blamed_author else ""
+        lines.append(
+            f"- **Likely culprit:** "
+            f"[`{blamed_commit[:12]}`]({_git_url(blamed_commit)}){by}"
+        )
+
     bug_id = findings.get("bug_id") or (run_doc.get("inputs") or {}).get("bug_id")
     if bug_id:
         lines.append(f"- **Bug:** [{bug_id}]({_bug_url(bug_id)})")
@@ -146,6 +178,8 @@ def _build_body(ctx: RunContext, run_doc: dict, patch: str | None = None) -> str
         run_url = f"{settings.hackbot_ui_url.rstrip('/')}/runs/{ctx.run_id}"
         lines.append(f"- **Run details:** {run_url}")
 
+    lines += _recipients_note(ctx, blamed_commit, blamed_author)
+
     if findings.get("summary"):
         lines += ["", "## Summary", "", _demote_headings(findings["summary"])]
     if findings.get("analysis"):
@@ -162,9 +196,49 @@ def _build_body(ctx: RunContext, run_doc: dict, patch: str | None = None) -> str
     if patch:
         lines += ["", "## Proposed patch", "", _patch_block(patch)]
 
+    if settings.notification_team_email:
+        lines += [
+            "",
+            "---",
+            "",
+            "_Reply to this email with any feedback on this analysis; it reaches "
+            "the hackbot team._",
+        ]
+
     return "\n".join(lines)
 
 
+def _recipients_note(
+    ctx: RunContext, blamed_commit: str | None, blamed_author: str | None
+) -> list[str]:
+    """Explain why each recipient is on the email.
+
+    The notification goes to the developer who pushed the failing change, the
+    author the agent blamed for the failure, and the team; spell out both roles
+    so the recipient list is self-explanatory.
+    """
+    notes: list[str] = []
+    if ctx.developer_email:
+        notes.append(
+            f"- **{ctx.developer_email}** pushed the change whose build failed."
+        )
+    if blamed_commit and blamed_author:
+        notes.append(
+            f"- **{blamed_author}** authored "
+            f"[`{blamed_commit[:12]}`]({_git_url(blamed_commit)}), which the "
+            "build-repair agent believes introduced the failure."
+        )
+    elif blamed_commit:
+        notes.append(
+            f"- The build-repair agent believes "
+            f"[`{blamed_commit[:12]}`]({_git_url(blamed_commit)}) introduced the "
+            "failure."
+        )
+    if not notes:
+        return []
+    return ["", "## Why you're receiving this", "", *notes]
+
+
 def _demote_headings(md: str, by: int = 2) -> str:
     """Shift ATX headings down ``by`` levels so agent docs nest under our own.
 

services/hackbot-pulse-listener/scripts/send_test_run.py

@@ -0,0 +1,90 @@
+"""Trigger a build-repair run for a real failing build task and email the result.
+
+Drives the listener's normal path (trigger the agent via hackbot-api, poll the
+run to completion, send the notification) from a synthetic pulse message, so you
+can test on a real failure without waiting for a live one.
+
+Credentials and settings are read from the environment / ``.env`` (see the
+service README): HACKBOT_API_URL, HACKBOT_API_KEY, SENDGRID_API_KEY,
+NOTIFICATION_SENDER, and NOTIFICATION_OVERRIDE_EMAIL. Always set
+NOTIFICATION_OVERRIDE_EMAIL to your own address so the run emails you and not the
+real developer; the script refuses to run otherwise.
+
+Usage (from the service directory, with the env exported):
+
+    uv run --package hackbot-pulse-listener python scripts/send_test_run.py \
+        <TASK_ID> --label build-linux64/opt [--project autoland] [--force]
+
+Find a task id on Treeherder: a red build ("B") job -> Task inspector -> taskId.
+"""
+
+import argparse
+import sys
+from concurrent.futures import ThreadPoolExecutor
+
+from app import consumer
+from app.config import settings
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("task_id", help="Taskcluster task id of a failed build job")
+    parser.add_argument(
+        "--label",
+        default="build-linux64/opt",
+        help="Build task label; must contain 'build' and not 'test'",
+    )
+    parser.add_argument("--project", default="autoland", help="Taskcluster project tag")
+    parser.add_argument(
+        "--created-for", default="", help="createdForUser: the pushing developer email"
+    )
+    parser.add_argument(
+        "--force",
+        action="store_true",
+        help="Skip the regression gate so a run always triggers",
+    )
+    args = parser.parse_args()
+
+    if not settings.notification_override_email:
+        parser.error(
+            "Set NOTIFICATION_OVERRIDE_EMAIL to your address so the test emails you, "
+            "not the real developer."
+        )
+
+    if args.force:
+        consumer.regression.is_new_build_failure = lambda *a, **k: True
+
+    if args.project not in settings.watched_repos_set:
+        settings.watched_repos = f"{settings.watched_repos},{args.project}"
+
+    msg = {
+        "status": {"taskId": args.task_id},
+        "task": {
+            "tags": {
+                "kind": "build",
+                "project": args.project,
+                "label": args.label,
+                "createdForUser": args.created_for,
+            }
+        },
+    }
+
+    with ThreadPoolExecutor(max_workers=4) as executor:
+        run_id = consumer.process(msg, executor)
+        if run_id is None:
+            print(
+                "No run triggered (filtered out, deduped, or DRY_RUN). Check "
+                "WATCHED_REPOS/DRY_RUN, or pass --force to skip the regression gate.",
+                file=sys.stderr,
+            )
+            return 1
+        print(
+            f"Triggered run {run_id}; polling until it finishes and emailing "
+            f"{settings.notification_override_email} (this can take several minutes)..."
+        )
+    print("Done.")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

services/hackbot-pulse-listener/tests/test_consumer.py

@@ -66,8 +66,8 @@ def test_build_failure_triggers_run_and_submits_poll():
     assert run_id == "run-1"
     trigger.assert_called_once()
     inputs = trigger.call_args.args[0]
-    assert inputs["git_commit"] == "deadbeef"
     assert inputs["failure_tasks"] == {"build-linux64/opt": "ABC"}
+    assert "git_commits" not in inputs
     executor.submit.assert_called_once()
     fn, ctx = executor.submit.call_args.args
     assert fn is consumer.worker.poll_and_notify
@@ -79,6 +79,23 @@ def test_build_failure_triggers_run_and_submits_poll():
     assert ctx.developer_email == "dev@mozilla.com"
 
 
+def test_only_failure_tasks_sent_to_agent():
+    executor = MagicMock()
+    with (
+        patch.object(consumer.taskcluster, "get_hg_revision", return_value="hgrev"),
+        patch.object(consumer.lando, "hg_to_git", return_value="deadbeef"),
+        patch.object(consumer.regression, "is_new_build_failure", return_value=True),
+        patch.object(consumer.client, "trigger_run", return_value="run-1") as trigger,
+    ):
+        consumer.process(_build_msg(), executor)
+
+    # The agent resolves the push (and its authors) itself; the listener
+    # only hands it the failing tasks.
+    inputs = trigger.call_args.args[0]
+    assert inputs["failure_tasks"] == {"build-linux64/opt": "ABC"}
+    assert "git_commit" not in inputs
+
+
 def test_same_revision_triggers_once():
     executor = MagicMock()
     with (