Skip to content
Open
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
7 changes: 7 additions & 0 deletions services/hackbot-api/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need notification_override_email?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s for testing. It redirects emails to a test inbox so we can verify that notifications work correctly.


# Server
port: int = 8080
environment: str = "development"
Expand Down
86 changes: 86 additions & 0 deletions services/hackbot-api/app/notifications.py
Original file line number Diff line number Diff line change
@@ -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("_", " ")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would mention the first part of the run ID, similar to the UI. If there is a bug id, I would mention it here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in839d796

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
26 changes: 23 additions & 3 deletions services/hackbot-api/app/routers/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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)
7 changes: 7 additions & 0 deletions services/hackbot-api/app/templates/run_completed.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<!doctype html>
<html>
<body>
<p>Your <strong>$label</strong> has <strong>$outcome</strong>.</p>
<p><a href="$url">Open the run</a></p>
</body>
</html>
65 changes: 65 additions & 0 deletions services/hackbot-api/tests/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
14 changes: 13 additions & 1 deletion services/hackbot-api/tests/test_finalize_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
83 changes: 83 additions & 0 deletions services/hackbot-api/tests/test_notifications.py
Original file line number Diff line number Diff line change
@@ -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'<a href="{url}">' in html_body
assert "<strong>timed out</strong>" 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