test-repair: skip notifications for intermittent failures

agents/test-repair/README.md

@@ -65,10 +65,14 @@ Stage 2:
 
 ## Slack notification
 
-Every run records a `slack.post_message` action carrying its verdict -- the
-recommendation, the classification and confidence, the failing job (linked to the
-push in Treeherder), the culprit or the candidates that could not be ruled out,
-and whether a patch is attached.
+A run whose verdict a sheriff has to act on records a `slack.post_message` action
+carrying it -- the recommendation, the classification and confidence, the failing job
+(linked to the push in Treeherder), the culprit or the candidates that could not be
+ruled out, and whether a patch is attached. A known intermittent -- `intermittent`
+classified `do_not_backout` -- is not posted: it asks nothing of a sheriff and is the
+majority verdict, so it would be noise. An intermittent recommending `rerun` is still
+posted, since the retrigger is the sheriff's to run. The hackbot team gets every
+verdict either way, by email from the pulse listener.
 
 The message is posted by the apply step, not from the run, so it is visible in the
 hackbot UI before it lands and is delivered at most once. `test-repair` opts into

agents/test-repair/hackbot_agents/test_repair/__main__.py

@@ -9,7 +9,7 @@
 from .agent import TestRepairResult
 from .config import SKIP_FIREFOX_BUILD, SLACK_CHANNEL
 from .logs import download_failure_logs
-from .notify import build_message, resolve_culprit_author
+from .notify import build_message, resolve_culprit_author, sheriff_action_required
 from .resolve import Investigation, resolve_investigation
 
 logger = logging.getLogger(__name__)
@@ -77,15 +77,19 @@ async def main(ctx: HackbotContext) -> TestRepairResult:
         publish_file=ctx.publish_file,
     )
 
-    # Notifications are active for all runs
-    message = build_message(
-        result,
-        investigation,
-        task_id=task_id,
-        run_id=ctx.run_id,
-        culprit_author=resolve_culprit_author(source_repo, result.culprit_commit),
-    )
-    record_message(ctx.actions, SLACK_CHANNEL, message)
+    if sheriff_action_required(result):
+        message = build_message(
+            result,
+            investigation,
+            task_id=task_id,
+            run_id=ctx.run_id,
+            culprit_author=resolve_culprit_author(source_repo, result.culprit_commit),
+        )
+        record_message(ctx.actions, SLACK_CHANNEL, message)
+    else:
+        logger.info(
+            "Verdict is %s; not notifying %s", result.classification, SLACK_CHANNEL
+        )
     return result
 
 

agents/test-repair/hackbot_agents/test_repair/notify.py

@@ -9,6 +9,8 @@
 then visible in the hackbot UI before it lands, and the apply step delivers it at
 most once (see ``hackbot_runtime.actions.slack``).
 
+Only verdicts a sheriff acts on are posted -- see :func:`sheriff_action_required`.
+
 A few lines of context, then the verdict in full. Every identifier a sheriff would
 otherwise have to look up -- revisions, task, bug, run -- is a link, the way the
 pulse listener's email does it
@@ -44,6 +46,19 @@
 }
 
 
+def sheriff_action_required(result: TestRepairResult) -> bool:
+    """Whether the verdict is one a sheriff has to act on.
+
+    A known intermittent asks for nothing -- no backout, no retrigger -- and it is the
+    majority verdict, so posting those is pure noise. ``rerun`` is not one of them: an
+    intermittent the agent could not confirm still asks for a retrigger.
+    """
+    return not (
+        result.classification == "intermittent"
+        and result.recommendation == "do_not_backout"
+    )
+
+
 def _link(url: str, label: str) -> str:
     return f"<{url}|{label}>"
 

agents/test-repair/tests/test_notify.py

@@ -1,5 +1,5 @@
 from hackbot_agents.test_repair.agent import TestRepairResult
-from hackbot_agents.test_repair.notify import build_message
+from hackbot_agents.test_repair.notify import build_message, sheriff_action_required
 from hackbot_agents.test_repair.resolve import (
     CommitRange,
     FailingGroup,
@@ -55,6 +55,36 @@ def _message(result=None, investigation=None, **kwargs):
     )
 
 
+def test_a_known_intermittent_is_not_worth_a_notification():
+    assert not sheriff_action_required(
+        _result(
+            classification="intermittent",
+            recommendation="do_not_backout",
+            culprit_commit=None,
+        )
+    )
+
+
+def test_an_unconfirmed_intermittent_still_asks_for_a_retrigger():
+    assert sheriff_action_required(
+        _result(
+            classification="intermittent",
+            recommendation="rerun",
+            culprit_commit=None,
+        )
+    )
+
+
+def test_every_regression_verdict_is_notified():
+    assert sheriff_action_required(_result())
+    assert sheriff_action_required(_result(recommendation="rerun"))
+    # No culprit survived, so there is nothing to back out -- but a regression the
+    # agent could not pin down is still a sheriff's problem.
+    assert sheriff_action_required(
+        _result(recommendation="do_not_backout", culprit_commit=None)
+    )
+
+
 def test_reports_the_verdict_and_its_context_in_five_lines():
     assert _message(culprit_author="standard8@mozilla.com").splitlines() == [
         "*test-repair: BACK OUT the culprit* (regression, confidence 0.7)",

services/hackbot-pulse-listener/README.md

@@ -53,8 +53,8 @@ Failed **build** tasks go to `build-repair`; failed **test** tasks go to `test-r
 7. **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`).
+   mirror and mails its author; test-repair mails only the team address
+   (`NOTIFICATION_TEAM_EMAIL`) -- sheriffs are notified by the agent in Slack instead.
 
 The dedupe caches, the daily budget and pending-run tracking are all in-memory, so
 a restart resets them.
@@ -98,9 +98,10 @@ 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. 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 only `TEST_REPAIR_NOTIFICATION_EMAIL` and the team address --
-never the culprit author or the pushing developer, though the culprit is still named
-in the body. Set
+address if set; test-repair mails only the team address -- never the culprit author or
+the pushing developer, though the culprit is still named in the body. Its verdicts are
+tracking for the hackbot team, so every verdict is mailed, intermittents included; what
+reaches sheriffs is the agent's Slack message, and only when they have to act. 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

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

@@ -69,10 +69,9 @@ class Settings(BaseSettings):
     # Email notifications (SendGrid)
     sendgrid_api_key: str | None = None
     notification_sender: str | None = None
-    # Team address CC'd on every notification alongside the revision author.
+    # Team address CC'd on every build-repair notification alongside the revision
+    # author, and the only recipient of test-repair verdicts.
     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).

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

@@ -17,9 +17,8 @@ def send_email(
 ) -> None:
     """Email the failure analysis. Only succeeded runs are notified.
 
-    Routes on the agent that produced the run: test-repair sends a
-    verdict-led body to the test-repair notification address; build-repair keeps its
-    existing behavior.
+    Routes on the agent that produced the run: test-repair sends a verdict-led body to
+    the hackbot team address; build-repair keeps its existing behavior.
 
     ``already_actioned`` is Treeherder's classification when a sheriff has already
     dealt with the failure.
@@ -70,10 +69,11 @@ def _send_test_repair_email(
     # test-repair verdicts are always notified (including do-not-backout verdicts), so
     # the build-repair notify_only_with_patch gate does not apply here.
     #
-    # The distribution list and the team address only. A verdict is a triage signal
-    # for the team, not something to mail at the developer whose commit the agent
-    # happens to blame -- the culprit is still named in the body.
-    recipients = _recipients(settings.test_repair_notification_email)
+    # The hackbot team address only: sheriffs are notified in Slack, by the agent, and
+    # only for the verdicts they act on, while the team gets every verdict here to track
+    # what the agent decided. Never the developer whose commit the agent happens to
+    # blame -- the culprit is still named in the body.
+    recipients = _recipients(settings.notification_team_email)
     if not recipients:
         logger.info(
             "No recipients for test-repair run %s; skipping notification", ctx.run_id
@@ -84,7 +84,7 @@ def _send_test_repair_email(
         return
 
     patch = _fetch_patch(ctx.run_id, run_doc)
-    # In the subject too, so a sheriff can skip it from the inbox.
+    # In the subject too, so it can be skipped from the inbox.
     prefix = "[already actioned] " if already_actioned else ""
     subject = (
         f"[test-repair] {prefix}{_banner(findings)} - "
@@ -146,9 +146,9 @@ def _recipients(primary: str | None, secondary: str | None = None) -> list[str]:
     """Recipients for a run, deduped and ordered by priority, team address last.
 
     build-repair puts the blamed commit's author first and the pushing developer
-    second; test-repair passes only its distribution address and so reaches no
-    individual. ``notification_override_email`` short-circuits to a single address so
-    local testing never mails real developers or the team.
+    second; test-repair passes only the team address and so reaches no individual.
+    ``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]
@@ -187,8 +187,7 @@ def _already_actioned_banner(reason: str | None) -> list[str]:
         return []
     return [
         f"> **Already actioned by a sheriff.** Treeherder now classifies this job as "
-        f"_{reason}_, so the tree has been dealt with and this analysis needs no "
-        f"action from a sheriff. It is sent for the developer's reland.",
+        f"_{reason}_, so the tree has been dealt with.",
         "",
     ]
 
@@ -243,7 +242,8 @@ def _build_test_repair_body(
         lines.append(f"- **Bug:** [{bug}]({_bug_url(bug)})")
 
     lines += _run_details(ctx) + _analysis_sections(findings) + _patch_section(patch)
-    lines += _patch_advice(patch) + _team_footer()
+    # No team footer: the team is the only recipient.
+    lines += _patch_advice(patch)
     return "\n".join(lines)
 
 

services/hackbot-pulse-listener/deploy.sh

@@ -41,8 +41,6 @@ PULSE_USER="${PULSE_USER:?set PULSE_USER (https://pulseguardian.mozilla.org)}"
 WATCHED_REPOS="${WATCHED_REPOS:-autoland}"
 NOTIFICATION_SENDER="${NOTIFICATION_SENDER:?set NOTIFICATION_SENDER (verified SendGrid sender)}"
 NOTIFICATION_TEAM_EMAIL="${NOTIFICATION_TEAM_EMAIL:-}"
-# Distribution address; primary recipient of test-repair verdicts.
-TEST_REPAIR_NOTIFICATION_EMAIL="${TEST_REPAIR_NOTIFICATION_EMAIL:-}"
 
 SA_NAME="${SA_NAME:-hackbot-pulse-listener-run}"
 SA_EMAIL="${SA_EMAIL:-${SA_NAME}@${PROJECT}.iam.gserviceaccount.com}"
@@ -106,7 +104,6 @@ ENV_VARS="${ENV_VARS},ENVIRONMENT=production"
 ENV_VARS="${ENV_VARS},PULSE_USER=${PULSE_USER},WATCHED_REPOS=${WATCHED_REPOS}"
 ENV_VARS="${ENV_VARS},NOTIFICATION_SENDER=${NOTIFICATION_SENDER}"
 ENV_VARS="${ENV_VARS},NOTIFICATION_TEAM_EMAIL=${NOTIFICATION_TEAM_EMAIL}"
-ENV_VARS="${ENV_VARS},TEST_REPAIR_NOTIFICATION_EMAIL=${TEST_REPAIR_NOTIFICATION_EMAIL}"
 
 gcloud beta run worker-pools deploy "${SERVICE}" \
   --image "${IMAGE}" \