diff --git a/services/hackbot-api/app/config.py b/services/hackbot-api/app/config.py index bd245620a2..013517992a 100644 --- a/services/hackbot-api/app/config.py +++ b/services/hackbot-api/app/config.py @@ -104,6 +104,13 @@ class Settings(BaseSettings): push_auth_audience: str = "" push_auth_service_account: str = "" + # Run-completion email to the requester (see app/notifications.py). The + # override replaces the recipient so a dev deployment never mails real people. + hackbot_ui_url: str = "http://localhost:3000" + sendgrid_api_key: str = "" + notification_sender: str = "" + notification_override_email: str = "" + # Server port: int = 8080 environment: str = "development" diff --git a/services/hackbot-api/app/notifications.py b/services/hackbot-api/app/notifications.py new file mode 100644 index 0000000000..9d6c43d039 --- /dev/null +++ b/services/hackbot-api/app/notifications.py @@ -0,0 +1,86 @@ +"""Email the person who requested a run when it reaches a terminal state. + +A run launched from the UI carries its requester's email. Once the run has +succeeded, failed or timed out, that address gets a short note with the +outcome and a link to the run page, so nobody has to keep a tab open. Runs +with no requester (triggered by webhooks) are skipped. + +This module only composes and delivers the message. It keeps no state, so +calling it once per run is the caller's job. Delivery is best-effort: a +failed send is logged, never raised. Only the requester is addressed; this +is a personal ping, not a report. +""" + +import asyncio +import logging +from pathlib import Path +from string import Template + +import sendgrid +from sendgrid.helpers.mail import From, HtmlContent, Mail, Subject, To + +from app.config import settings +from app.database.models import Run + +log = logging.getLogger(__name__) + +# A stalled SendGrid call must not hold finalization open. +_SEND_TIMEOUT_SECONDS = 10 + +_TEMPLATES = Path(__file__).parent / "templates" +_HTML_TEMPLATE = Template((_TEMPLATES / "run_completed.html").read_text()) + + +def run_url(run_id: str) -> str: + return f"{settings.hackbot_ui_url.rstrip('/')}/runs/{run_id}" + + +def build_message(run: Run) -> tuple[str, str]: + """Compose the subject and HTML body of the notice for ``run``.""" + outcome = run.status.replace("_", " ") + label = f"{run.agent} run {str(run.run_id)[:8]}" + bug_id = run.inputs.get("bug_id") + if bug_id is not None: + label += f" for bug {bug_id}" + subject = f"[Hackbot] {label} {outcome}" + + values = {"label": label, "outcome": outcome, "url": run_url(str(run.run_id))} + html_body = _HTML_TEMPLATE.substitute(values) + return subject, html_body + + +def _recipient(run: Run) -> str: + return settings.notification_override_email.strip() or run.requested_by + + +def _send_sync(recipient: str, subject: str, html_body: str) -> int: + message = Mail( + From(settings.notification_sender), + To(recipient), + Subject(subject), + html_content=HtmlContent(html_body), + ) + client = sendgrid.SendGridAPIClient(api_key=settings.sendgrid_api_key) + # The SendGrid wrapper has no timeout option; its HTTP client does. + client.client.timeout = _SEND_TIMEOUT_SECONDS + response = client.send(message=message) + return response.status_code + + +async def notify_requester(run: Run) -> bool: + """Mail the run's requester about its terminal state. Returns whether it sent.""" + recipient = _recipient(run) + subject, html_body = build_message(run) + try: + status_code = await asyncio.to_thread(_send_sync, recipient, subject, html_body) + except Exception: + log.exception("Failed to notify %s about run %s", recipient, run.run_id) + return False + log.info( + "Notified %s about run %s (%s): SendGrid %s", + recipient, + run.run_id, + run.status, + status_code, + ) + return True diff --git a/services/hackbot-api/app/routers/events.py b/services/hackbot-api/app/routers/events.py index d46da7ab6d..a31935a494 100644 --- a/services/hackbot-api/app/routers/events.py +++ b/services/hackbot-api/app/routers/events.py @@ -7,6 +7,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from app import notifications from app.actions_applier import on_run_completed from app.auth import require_push_auth from app.database.connection import get_db @@ -24,9 +25,8 @@ def _decode_pubsub_push_body(body: dict) -> dict: """Decode a standard Pub/Sub push envelope's `message.data` as JSON. - Both the completion-log push subscription feeding agent-run-finished and the - `agent-run-events` action-applier subscription deliver via this same - envelope shape. + The completion-log, action-applier and requester-notification subscriptions + deliver via this same envelope shape. """ message = body.get("message") or {} data = message.get("data") @@ -135,3 +135,23 @@ async def apply_run_actions( return await on_run_completed(db, run) + + +@router.post("/notify-requester", status_code=204) +async def notify_requester( + request: Request, db: AsyncSession = Depends(get_db) +) -> None: + """Consumer of `run.completed`: email the run's requester. + + Its own subscription includes all terminal outcomes. + """ + event = _decode_pubsub_push_body(await request.json()) + run_id = event["run_id"] + + run = await db.get(Run, uuid.UUID(run_id)) + if run is None: + log.warning("No run found for run_id %s", run_id) + return + + if run.requested_by: + await notifications.notify_requester(run) diff --git a/services/hackbot-api/app/templates/run_completed.html b/services/hackbot-api/app/templates/run_completed.html new file mode 100644 index 0000000000..a6a765b237 --- /dev/null +++ b/services/hackbot-api/app/templates/run_completed.html @@ -0,0 +1,7 @@ + + +
+Your $label has $outcome.
+ + + diff --git a/services/hackbot-api/tests/test_events.py b/services/hackbot-api/tests/test_events.py index 214ca8e912..336e01a8a7 100644 --- a/services/hackbot-api/tests/test_events.py +++ b/services/hackbot-api/tests/test_events.py @@ -6,7 +6,13 @@ import base64 import json +import uuid +from types import SimpleNamespace +import pytest +from app import notifications +from app.auth import require_push_auth +from app.database.models import Run from app.routers.events import ( _decode_pubsub_push_body, _execution_name_from_completion_log, @@ -73,3 +79,62 @@ def test_execution_name_falls_back_to_labels(): def test_execution_name_missing(): assert _execution_name_from_completion_log({"protoPayload": {}}) is None assert _execution_name_from_completion_log({}) is None + + +@pytest.mark.parametrize("status", ["succeeded", "failed", "timed_out"]) +def test_notify_requester_consumes_completed_event(client, db, monkeypatch, status): + run = SimpleNamespace( + run_id=uuid.uuid4(), status=status, requested_by="someone@mozilla.com" + ) + notified = [] + + async def get(model, key): + assert model is Run + assert key == run.run_id + return run + + async def notify(value): + notified.append(value) + return True + + monkeypatch.setattr(db, "get", get) + monkeypatch.setattr(notifications, "notify_requester", notify) + client.app.dependency_overrides[require_push_auth] = lambda: None + response = client.post( + "/internal/events/notify-requester", + json=_push_envelope({"run_id": str(run.run_id), "status": status}), + ) + assert response.status_code == 204 + assert notified == [run] + + +def test_notify_requester_skips_missing_run(client, monkeypatch): + async def notify(run): + pytest.fail("No email should be sent without a run") + + monkeypatch.setattr(notifications, "notify_requester", notify) + client.app.dependency_overrides[require_push_auth] = lambda: None + response = client.post( + "/internal/events/notify-requester", + json=_push_envelope({"run_id": str(uuid.uuid4())}), + ) + assert response.status_code == 204 + + +def test_notify_requester_skips_run_without_requester(client, db, monkeypatch): + run = SimpleNamespace(run_id=uuid.uuid4(), requested_by=None) + + async def get(model, key): + return run + + async def notify(value): + pytest.fail("No email should be sent without a requester") + + monkeypatch.setattr(db, "get", get) + monkeypatch.setattr(notifications, "notify_requester", notify) + client.app.dependency_overrides[require_push_auth] = lambda: None + response = client.post( + "/internal/events/notify-requester", + json=_push_envelope({"run_id": str(run.run_id)}), + ) + assert response.status_code == 204 diff --git a/services/hackbot-api/tests/test_finalize_run.py b/services/hackbot-api/tests/test_finalize_run.py index 0ed627902e..7edfc6b006 100644 --- a/services/hackbot-api/tests/test_finalize_run.py +++ b/services/hackbot-api/tests/test_finalize_run.py @@ -10,7 +10,7 @@ from datetime import datetime, timezone import pytest -from app import gcs, jobs, pubsub +from app import gcs, jobs, notifications, pubsub from app.jobs import ExecutionStatus from app.routers import runs as runs_module from app.routers.runs import finalize_run @@ -48,6 +48,18 @@ async def fake_publish(run_id, agent, status): return published +@pytest.fixture(autouse=True) +def _no_notify(monkeypatch): + notified = [] + + async def fake_notify(run): + notified.append(run) + return True + + monkeypatch.setattr(notifications, "notify_requester", fake_notify) + return notified + + async def test_noop_when_already_finalized(monkeypatch): run = _FakeRun(finalized_at=datetime.now(timezone.utc)) db = _FakeDB() diff --git a/services/hackbot-api/tests/test_notifications.py b/services/hackbot-api/tests/test_notifications.py new file mode 100644 index 0000000000..935828829a --- /dev/null +++ b/services/hackbot-api/tests/test_notifications.py @@ -0,0 +1,83 @@ +"""Tests for the run-completion email to the requester (app/notifications.py).""" + +import uuid +from dataclasses import dataclass, field + +import pytest +from app import notifications +from app.config import settings +from app.notifications import build_message, notify_requester +from app.schemas import RunStatus + + +@dataclass +class _FakeRun: + run_id: uuid.UUID = field(default_factory=uuid.uuid4) + agent: str = "bug-fix" + status: str = RunStatus.succeeded.value + requested_by: str | None = "someone@mozilla.com" + inputs: dict = field(default_factory=dict) + + +@pytest.fixture +def sent(monkeypatch): + """Capture outgoing mail instead of hitting SendGrid.""" + calls = [] + + def fake_send(recipient, subject, html_body): + calls.append((recipient, subject, html_body)) + return 202 + + monkeypatch.setattr(notifications, "_send_sync", fake_send) + monkeypatch.setattr(settings, "sendgrid_api_key", "sg-test") + monkeypatch.setattr(settings, "notification_sender", "hackbot@mozilla.com") + monkeypatch.setattr(settings, "notification_override_email", "") + return calls + + +def test_build_message_links_to_run_page(monkeypatch): + monkeypatch.setattr(settings, "hackbot_ui_url", "https://ui.example/") + run = _FakeRun(status=RunStatus.timed_out.value) + subject, html_body = build_message(run) + url = f"https://ui.example/runs/{run.run_id}" + assert subject == f"[Hackbot] bug-fix run {str(run.run_id)[:8]} timed out" + assert f"bug-fix run {str(run.run_id)[:8]}" in html_body + assert f'' in html_body + assert "timed out" in html_body + + +async def test_sends_to_requester(sent): + run = _FakeRun() + assert await notify_requester(run) is True + assert len(sent) == 1 + recipient, subject, _ = sent[0] + assert recipient == "someone@mozilla.com" + assert subject == f"[Hackbot] bug-fix run {str(run.run_id)[:8]} succeeded" + + +def test_build_message_includes_bug_id(): + run = _FakeRun( + run_id=uuid.UUID("ab603010-c278-4d55-bc29-f89463f78906"), + inputs={"bug_id": 123456}, + ) + subject, html_body = build_message(run) + assert subject == "[Hackbot] bug-fix run ab603010 for bug 123456 succeeded" + assert "bug-fix run ab603010 for bug 123456" in html_body + + +async def test_override_email_replaces_recipient(sent, monkeypatch): + monkeypatch.setattr(settings, "notification_override_email", "dev@example.com") + assert await notify_requester(_FakeRun()) is True + assert sent[0][0] == "dev@example.com" + + +async def test_send_failure_is_logged_not_raised(monkeypatch, caplog): + monkeypatch.setattr(settings, "sendgrid_api_key", "sg-test") + monkeypatch.setattr(settings, "notification_sender", "hackbot@mozilla.com") + + def boom(*_a): + raise RuntimeError("sendgrid down") + + monkeypatch.setattr(notifications, "_send_sync", boom) + assert await notify_requester(_FakeRun()) is False + assert "Failed to notify" in caplog.text