[WIP] Test repair agent Part 2: update the pulse listener

services/hackbot-pulse-listener/README.md

@@ -1,54 +1,71 @@
 # Hackbot Pulse Listener
 
-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 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.
+Its job is to **subscribe** to Taskcluster failure messages, **filter** them down to
+failures worth acting on, **dedupe** them, and dispatch a hackbot agent through the
+hackbot-api. It deliberately holds no investigation logic: each agent resolves the push
+and commits itself, so the listener only decides _what to hand off_. When a run finishes
+(minutes later) the listener polls the result and emails a report.
+
+Failed **build** tasks go to `build-repair`; failed **test** tasks go to `test-repair`.
 
 ## How it works
 
-1. Consume `task-failed` messages from `pulse.mozilla.org`.
-2. Keep only **build** tasks (`tags.label` contains `build` and not `test`) on a watched
-   `project` (`WATCHED_REPOS`, default `autoland`). Build tasks don't run tests, so a
-   failure is a compilation/link error.
-3. Fetch the task definition to read `GECKO_HEAD_REV` (the revision is not in the message).
-4. Skip tasks scheduled by an **action task** rather than by the push: `extra.parent`
-   points at the decision task (= the task group) for everything the push scheduled, and at
-   the action-callback task for a backfill or retrigger.
-5. Skip pushes that landed more than `MAX_PUSH_AGE_HOURS` ago (default 24). A failure can
-   surface long after its push, and by then the push has been superseded.
-6. Skip failures **inherited** from an ancestor push, waiting for an unsettled ancestor
-   build to finish first. Both this and the push-age check fail open on error, so a real
-   regression is never silently dropped.
-7. 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.
-8. 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.
-9. `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).
+1. **Subscribe.** Consume `task-failed` messages from `pulse.mozilla.org`.
+2. **Route** to a watched `project` (`WATCHED_REPOS`, default `autoland`), then by task
+   kind: build tasks (`tags.label` contains `build` and not `test`, so a failure is a
+   compilation/link error) take the build-repair path; test tasks take the test-repair
+   path. Fetch the task definition for `GECKO_HEAD_REV` (not in the message).
+3. **Discard what is not this push's failure**, on both paths:
+   - Tasks scheduled by an **action task** rather than by the push: `extra.parent` points
+     at the decision task (= the task group) for everything the push scheduled, and at the
+     action-callback task for a backfill or retrigger.
+   - Pushes that landed more than `MAX_PUSH_AGE_HOURS` ago (default 24). A failure can
+     surface long after its push, and by then the push has been superseded.
+4. **Dedupe** with in-memory TTL caches, both keyed by revision: one run per push per
+   agent, triggered on the first failing task worth investigating. The test-repair
+   agent works from a single task but reads the push's other failures itself, so a
+   second run for the same push would re-tread the same ground. A revision is recorded
+   only once a run is actually triggered, so a task rejected as intermittent or
+   inherited leaves the push open for the next one.
+5. **Judge** whether the failure is worth a run. Every check fails open — an upstream
+   error runs the agent rather than dropping a possible regression.
+   - _Build:_ keep only failures this push introduced, waiting for an unsettled ancestor
+     build to finish first.
+   - _Test:_ first drop whatever Treeherder judges not to be a new regression
+     (intermittent, infra, expected-fail, fixed-by-commit). Treeherder ingests a minute or
+     so behind us and classifies a few minutes after that, so this gate waits for the job
+     to appear and then for its verdict — it is the cheap filter, and most test failures
+     stop here, before any group is fetched or any ancestor walked. What survives is
+     narrowed to the groups that are new for this task's own configuration (platform and
+     build option), then Treeherder is asked once more, since a verdict can still land
+     while that walk runs. The run carries only the task id.
+6. **Dispatch & report.** `POST /agents/{agent}/runs`, poll `GET /runs/{run_id}` until
+   terminal, then email a hackbot UI link, the analysis summary, a Treeherder link, and the
+   commit the agent blamed. Build-repair looks the blamed commit up in the firefox GitHub
+   mirror and mails its author; test-repair mails the notification address
+   (`TEST_REPAIR_NOTIFICATION_EMAIL`).
+
+The dedupe caches and pending-run tracking are in-memory (reset on restart).
 
 ## Run locally
 
 ```bash
 export PULSE_USER=... PULSE_PASSWORD=...          # https://pulseguardian.mozilla.org
 export HACKBOT_API_URL=https://hackbot-api.../ HACKBOT_API_KEY=...
 export HACKBOT_UI_URL=https://hackbot-ui.../
-export WATCHED_REPOS=try
+export WATCHED_REPOS=autoland
 export DRY_RUN=true                               # log intended calls, don't POST
 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. 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.
+is logged and skipped. Build-repair mails the blamed commit's author (looked up in the
+firefox GitHub mirror), the pushing developer, and the `NOTIFICATION_TEAM_EMAIL` team
+address if set; test-repair mails `TEST_REPAIR_NOTIFICATION_EMAIL` (with the culprit author CC'd). Set
+`NOTIFICATION_OVERRIDE_EMAIL` to route every notification to a single address (useful for
+local testing). By default only build-repair runs that produced a patch are emailed; set
+`NOTIFY_ONLY_WITH_PATCH=false` to also notify on transient / not-to-blame runs (test-repair always
+notifies).
 When `NOTIFICATION_TEAM_EMAIL` is set, notifications use it as `Reply-To` so recipients can
 reply with feedback on the analysis.
 

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

@@ -1,11 +1,13 @@
 import logging
+import os
 import signal
 from concurrent.futures import ThreadPoolExecutor
 
+# Configures logging on import; must precede the imports that pull in mozci.
+import app.logging_setup  # noqa: F401
 from app import consumer
 from app.config import settings
 
-logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
 
 
@@ -30,14 +32,35 @@ def shutdown(signum, _frame):
     signal.signal(signal.SIGINT, shutdown)
 
     logger.info(
-        "Listening for build failures on %s; watched repos: %s",
+        "Listening for build and test failures on %s; watched repos: %s%s",
         ", ".join(consumer.EXCHANGES),
         sorted(settings.watched_repos_set),
+        " (DRY RUN: no agent runs will be triggered)" if settings.dry_run else "",
     )
+    code = 0
     try:
         consumer_obj.run()
+    except Exception:
+        logger.exception("Consumer loop failed")
+        code = 1
     finally:
-        executor.shutdown(wait=False)
+        _exit_now(executor, code)
+
+
+def _exit_now(executor: ThreadPoolExecutor, code: int) -> None:
+    """Drop in-flight work and exit immediately.
+
+    Worker threads block for a long time by design (a regression check waits up
+    to an hour for an ancestor push, run polling up to ``run_max_age_minutes``),
+    and ThreadPoolExecutor's threads are non-daemon: its atexit hook joins them,
+    so a normal exit would hang for hours and Ctrl+C would appear to do nothing.
+    That hook also breaks any thread still submitting to a pool mid-shutdown.
+    In-flight work is disposable (pending runs are only tracked in memory), so
+    abandon it rather than wait.
+    """
+    executor.shutdown(wait=False, cancel_futures=True)
+    logging.shutdown()
+    os._exit(code)
 
 
 if __name__ == "__main__":

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

@@ -13,13 +13,23 @@ def _headers() -> dict[str, str]:
     return {"X-API-Key": settings.hackbot_api_key}
 
 
-def trigger_run(inputs: dict) -> str | None:
-    """Create a build-repair run. Returns the run id, or None in dry-run mode."""
+def trigger_run(inputs: dict, agent_name: str | None = None) -> str | None:
+    """Create an agent run. Returns the run id, or None in dry-run mode.
+
+    The model/turn overrides are settings shared by every agent, so they are added
+    here rather than by each caller.
+    """
+    agent = agent_name or settings.agent_name
+    inputs = dict(inputs)
+    if settings.model:
+        inputs["model"] = settings.model
+    if settings.max_turns is not None:
+        inputs["max_turns"] = settings.max_turns
     if settings.dry_run:
-        logger.info("[dry-run] would trigger %s run: %s", settings.agent_name, inputs)
+        logger.info("[dry-run] would trigger %s run: %s", agent, inputs)
         return None
 
-    url = f"{settings.hackbot_api_url}/agents/{settings.agent_name}/runs"
+    url = f"{settings.hackbot_api_url}/agents/{agent}/runs"
     resp = httpx.post(url, json=inputs, headers=_headers(), timeout=_TIMEOUT)
     resp.raise_for_status()
     return resp.json()["run_id"]

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

@@ -12,6 +12,8 @@ class Settings(BaseSettings):
     hackbot_api_key: str = ""
     hackbot_ui_url: str = ""
     agent_name: str = "build-repair"
+    # Agent that analyzes test failures (separate Cloud Run Job from build-repair).
+    test_repair_agent_name: str = "test-repair"
 
     # Source links shown in notifications.
     firefox_git_url: str = "https://github.com/mozilla-firefox/firefox"
@@ -25,11 +27,19 @@ class Settings(BaseSettings):
     # Pushes older than this are not repaired. A build failure can arrive long
     # after its push (a backfill, a long-queued task, a replayed message), and by
     # then the push has been superseded. Generous next to the minutes a build
-    # needs plus the hour the regression check may wait for an ancestor.
+    # needs plus the ten the regression check may wait for an ancestor.
     max_push_age_hours: float = 24
     run_try_push: bool = False
     model: str | None = None
     max_turns: int | None = None
+    # Treeherder classifies a failing job shortly after we see the failure, so the
+    # gate waits for the job to be ingested before reading that verdict.
+    treeherder_ingest_poll_seconds: int = 30
+    treeherder_ingest_max_wait_seconds: int = 240
+    # How long to wait for a verdict once the job is ingested. Classification lands a
+    # few minutes after ingestion, and most test failures turn out to be intermittent
+    # or expected-fail, so waiting here rejects them before the ancestor walk.
+    treeherder_classification_wait_seconds: int = 300
 
     # Dedupe (in-memory, by hg revision)
     dedupe_ttl_seconds: int = 6 * 60 * 60
@@ -50,12 +60,18 @@ class Settings(BaseSettings):
     notification_sender: str | None = None
     # Team address CC'd on every notification alongside the revision author.
     notification_team_email: str | None = None
+    # Distribution address; primary recipient of test-repair verdicts.
+    test_repair_notification_email: str | None = None
     # Send all notifications to this address instead of the developer (local testing).
     notification_override_email: str | None = None
     # Only notify when the run produced a patch (skip transient / not-to-blame runs).
     notify_only_with_patch: bool = True
 
     dry_run: bool = False
+    log_level: str = "INFO"
+    # mozci's own (loguru) logging. Its per-task "missing results" warnings are
+    # normal operation, not something to act on; lower this to debug mozci.
+    mozci_log_level: str = "ERROR"
     environment: str = "development"
     sentry_dsn: str | None = None