feat(alerts): alert email waits 2 minutes in the outbox and each alert records its outcome (#5507)

#5507 "Alert emails through the outbox, gathered for 2 minutes"
(milestone 501). Replaces the in-memory core/alert_burst.py.

- outbox.hold_alert: an alert joins each recipient's held alert email, or
  opens one due 2 minutes later (System 156 ruling). A Postgres advisory
  lock keeps two alerts at once from opening two windows for one reader.
- email_outbox_events.alert_json (migration 0043) keeps the alert as it
  fired, so the email written when the window closes says what it said.
- write_held writes it then, in the reader's zone: the usual alert email
  for one alert, the list for several (outbox.alert_words).
- after_settle records on each AlertEvent, once every email carrying it
  has settled: email_sent, or email_error naming each reader it didn't
  reach and the server's reason.
- A held email survives a restart and goes out when its window closes.
- A refusal of a message's one recipient no longer repeats the address.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

steward/core/alert_burst.py

@@ -1,105 +0,0 @@
-"""Alert emails gathered into one message per burst (#5465).
-
-When the network drops or a switch reboots, every host's agent-offline rule
-fires together, and one email per rule per account turns one incident into a
-dozen messages. So alert email waits: the first alert opens a window of
-BURST_WINDOW_S, and when it closes each recipient gets one email for every
-alert that arrived in it. An alert on its own still reads as it always has.
-
-Webhooks are not held here: each alert posts on its own, at once, since an
-integration expects one alert per post (core.alerts._dispatch_notification).
-Every alert's event still records the email's outcome.
-
-A burst held when Steward stops is not sent; the alerts stay on the record
-and on the Alerts page.
-"""
-from __future__ import annotations
-
-import asyncio
-import logging
-from typing import TYPE_CHECKING
-
-from sqlalchemy import select
-
-if TYPE_CHECKING:
-    from quart import Quart
-
-logger = logging.getLogger(__name__)
-
-# Long enough to gather what one incident sets off (the agent-offline check
-# runs each minute, agents report every 10-60 s), short enough that a lone
-# alert is still news when it lands.
-BURST_WINDOW_S = 30.0
-
-_held: list[tuple[dict, str]] = []          # (alert data, event id), oldest first
-_flusher: asyncio.Task | None = None
-
-
-def hold(app: "Quart", alert: dict, event_id: str) -> None:
-    """Add an alert to the email burst, opening a window if none is open."""
-    global _flusher
-    _held.append((alert, event_id))
-    if _flusher is None or _flusher.done():
-        _flusher = asyncio.create_task(_after_window(app))
-
-
-async def _after_window(app: "Quart") -> None:
-    """Wait out the window, then send what it gathered. Alerts that arrive
-    while the burst is being sent open the next window."""
-    global _flusher
-    try:
-        await asyncio.sleep(BURST_WINDOW_S)
-        await flush(app)
-    except Exception:
-        logger.exception("Alert email burst failed")
-    finally:
-        _flusher = None
-        if _held:
-            _flusher = asyncio.create_task(_after_window(app))
-
-
-async def flush(app: "Quart") -> None:
-    """Send every held alert now: one email per recipient, then record the
-    outcome on each alert's event."""
-    from steward.core.mail import recipients
-    items = _held[:]
-    _held.clear()
-    if not items:
-        return
-    smtp = app.config.get("SMTP", {})
-    if not smtp.get("host"):
-        return
-    async with app.db_sessionmaker() as db:
-        to = await recipients(db, "alerts", smtp)
-    result = await email_burst(smtp, [alert for alert, _ in items], to)
-    if result is not None:
-        await _record(app, [event_id for _, event_id in items], result)
-
-
-async def email_burst(cfg: dict, alerts: list[dict], to: list) -> dict | None:
-    """Email `alerts` to each recipient as one message, in their own zone:
-    the usual alert email for one alert, a list for several. Returns
-    {"sent", "error"}, or None when there is nobody to send to."""
-    from steward.core.mail import alert_email, burst_email, message, send
-    if not to or not alerts:
-        return None
-    try:
-        messages = []
-        for r in to:
-            parts = alert_email(cfg, alerts[0], r) if len(alerts) == 1 else burst_email(cfg, alerts, r)
-            messages.append(message(cfg, r.address, *parts))
-        await send(cfg, messages)
-        return {"sent": True, "error": None}
-    except Exception as exc:
-        logger.error("Alert email failed: %s", exc)
-        return {"sent": False, "error": str(exc)}
-
-
-async def _record(app: "Quart", event_ids: list[str], result: dict) -> None:
-    from steward.models.alerts import AlertEvent
-    async with app.db_sessionmaker() as db:
-        async with db.begin():
-            for event in (await db.execute(
-                    select(AlertEvent).where(AlertEvent.id.in_(event_ids)))).scalars():
-                event.email_sent = result["sent"]
-                event.email_error = result.get("error")

steward/core/alerts.py

@@ -495,9 +495,10 @@ async def _dispatch_notification(
     event_id: str,
     note: str | None = None,
 ) -> None:
-    """Run after the transaction commits. Holds the alert's email for its
-    burst (core.alert_burst), posts its webhook, and records the webhook's
-    outcome on the event; the burst records the email's."""
+    """Run after the transaction commits. Queues the alert's email in the
+    outbox, gathered for 2 minutes with any others (core.outbox.hold_alert),
+    posts its webhook, and records the webhook's outcome on the event; the
+    outbox records the email's once it settles."""
     from steward.core.notifications import dispatch_notifications
 
     alert_data = {
@@ -513,11 +514,14 @@ async def _dispatch_notification(
         "timestamp": datetime.now(timezone.utc).isoformat(),
     }
 
-    # Email waits a moment so alerts that arrive together go out as one
-    # message per recipient (#5465); the webhook posts this alert now.
-    from steward.core import alert_burst
+    # Email waits so alerts that arrive together go out as one message per
+    # recipient (#5465, #5507); the webhook posts this alert now.
+    from steward.core import outbox
     if app.config.get("SMTP", {}).get("host"):
-        alert_burst.hold(app, alert_data, event_id)
+        try:
+            await outbox.hold_alert(app, alert_data, event_id)
+        except Exception:
+            logger.exception("Alert %s: its email could not be queued", event_id)
     results = await dispatch_notifications({}, app.config.get("WEBHOOK", {}), alert_data)
 
     webhook_r = results.get("webhook")

steward/core/mail.py

@@ -252,7 +252,7 @@ def alert_email(cfg: dict, alert: dict, to: Recipient) -> tuple[str, str, str]:
 
 def burst_email(cfg: dict, alerts: list[dict], to: Recipient) -> tuple[str, str, str]:
     """(subject, text, html) for several alerts that arrived together
-    (core.alert_burst, #5465): how many and what they came to in the subject,
+    (core.outbox.alert_words, #5465): how many and what they came to in the subject,
     then each alert's headline, rule and time, oldest first, in the
     recipient's zone."""
     from collections import Counter

steward/core/outbox.py

@@ -16,21 +16,28 @@ How a row settles:
   by then, it is failed with the last error.
 - **a test** (retry=False): settles on its first answer either way.
 
+Alert email (#5507) is held: the first alert opens a row per recipient with
+send_after ALERT_WINDOW_S out, alerts arriving before it closes join the
+same rows, and the email is written when the window closes: the usual alert
+email for one, a list for several. Each alert's event records the outcome
+once every email carrying it has settled.
+
 Nothing is kept in memory. A row left behind by a restart is still pending
 (or held, or claimed by a run that died), and the next tick picks it up.
 """
 from __future__ import annotations
 
 import asyncio
+import json
 import logging
 import smtplib
 from datetime import datetime, timedelta, timezone
 from email.message import EmailMessage
 from typing import TYPE_CHECKING
 
-from sqlalchemy import or_, select, update
+from sqlalchemy import insert, or_, select, text, update
 
-from steward.models.outbox import EmailOutbox
+from steward.models.outbox import EmailOutbox, email_outbox_events
 
 if TYPE_CHECKING:
     from quart import Quart
@@ -47,6 +54,10 @@ GIVE_UP_S = 3600
 # A claim older than this belongs to a run that died mid-send: back to pending.
 STALE_CLAIM_S = 300
 BATCH = 50
+# Alerts that fire within this of the first go out as one email (System 156
+# ruling: "change the timing on 3 to be 2 minutes"). One incident's alerts
+# arrive over a minute or so: the agent-offline check runs each minute.
+ALERT_WINDOW_S = 120
 
 
 def backoff(attempts: int) -> int:
@@ -76,6 +87,9 @@ def reason(exc: BaseException) -> str:
         text = msg.decode(errors="replace") if isinstance(msg, bytes) else str(msg)
         return f"{code} {' '.join(text.split())}"
     if isinstance(exc, smtplib.SMTPRecipientsRefused):
+        # Each message has one reader, whose row already names them.
+        if len(exc.recipients) == 1:
+            return said(*next(iter(exc.recipients.values())))
         return "; ".join(f"{to}: {said(c, m)}" for to, (c, m) in exc.recipients.items())
     if isinstance(exc, smtplib.SMTPResponseException):
         return said(exc.smtp_code, exc.smtp_error)
@@ -148,10 +162,61 @@ def deliver_sync(cfg: dict, messages: list[EmailMessage]) -> list[BaseException
     return outcomes + [smtplib.SMTPServerDisconnected("connection lost")] * (len(messages) - len(outcomes))
 
 
-async def write_held(app: "Quart", row: EmailOutbox) -> None:
-    """Fill in a row queued without its words: an alert burst, written when
-    its window closes (#5507). Raises when it can't be written."""
-    raise ValueError(f"no words to send for a {row.kind} email")
+async def hold_alert(app: "Quart", alert: dict, event_id: str,
+                     now: datetime | None = None) -> int:
+    """Queue an alert's email: join each recipient's open alert email, or
+    open one that waits ALERT_WINDOW_S. Returns how many it went to. The
+    event must be committed first (the link refers to it)."""
+    from steward.core.mail import recipients
+    cfg = app.config.get("SMTP", {})
+    now = now or datetime.now(timezone.utc)
+    async with app.db_sessionmaker() as db:
+        async with db.begin():
+            # One holder at a time, so two alerts at once can't each open a
+            # window for the same reader.
+            await db.execute(text("SELECT pg_advisory_xact_lock(hashtext('steward.email_outbox.alert'))"))
+            to = await recipients(db, "alerts", cfg)
+            if not to:
+                return 0
+            open_rows = {r.recipient: r for r in (await db.execute(
+                select(EmailOutbox).where(EmailOutbox.kind == "alert", EmailOutbox.state == "held",
+                                          EmailOutbox.recipient.in_([r.address for r in to]))
+                .with_for_update())).scalars()}
+            rows = [open_rows.get(r.address) or enqueue(
+                db, kind="alert", to=r, send_after=now + timedelta(seconds=ALERT_WINDOW_S), now=now)
+                for r in to]
+            await db.flush()
+            await db.execute(insert(email_outbox_events), [
+                {"outbox_id": row.id, "event_id": event_id, "alert_json": json.dumps(alert)}
+                for row in rows])
+    return len(to)
+
+
+def alert_words(cfg: dict, alerts: list[dict], to: "Recipient") -> tuple[str, str, str]:
+    """(subject, text, html) for the alerts one email carries, oldest first:
+    the usual alert email for one, a list of them for several (#5465)."""
+    from steward.core.mail import alert_email, burst_email
+    return alert_email(cfg, alerts[0], to) if len(alerts) == 1 else burst_email(cfg, alerts, to)
+
+
+async def write_held(app: "Quart", db, row: EmailOutbox) -> None:
+    """Fill in a row queued without its words: an alert email, written when
+    its window closes, in its reader's zone. Raises when it can't be."""
+    from zoneinfo import ZoneInfo
+
+    from steward.core.mail import Recipient
+    from steward.models.alerts import AlertEvent
+    if row.kind != "alert":
+        raise ValueError(f"no words to send for a {row.kind} email")
+    alerts = [json.loads(a) for a in (await db.execute(
+        select(email_outbox_events.c.alert_json)
+        .join(AlertEvent, AlertEvent.id == email_outbox_events.c.event_id)
+        .where(email_outbox_events.c.outbox_id == row.id)
+        .order_by(AlertEvent.transitioned_at, AlertEvent.id))).scalars()]
+    if not alerts:
+        raise ValueError("The alerts this email carried were deleted")
+    to = Recipient(row.recipient, ZoneInfo(row.tz), row.show_utc)
+    row.subject, row.text, row.html = alert_words(app.config.get("SMTP", {}), alerts, to)
 
 
 async def attempt(app: "Quart", ids: list[str], now: datetime | None = None) -> dict[str, str]:
@@ -172,7 +237,7 @@ async def attempt(app: "Quart", ids: list[str], now: datetime | None = None) ->
                 continue
             try:
                 if row.subject is None:
-                    await write_held(app, row)
+                    await write_held(app, db, row)
                 sendable.append((row.id, message(cfg, row.recipient, row.subject, row.text, row.html)))
             except Exception as exc:
                 logger.exception("email %s: could not be written", row.id)
@@ -196,8 +261,30 @@ async def attempt(app: "Quart", ids: list[str], now: datetime | None = None) ->
 
 
 async def after_settle(db, rows: list[EmailOutbox]) -> None:
-    """What else follows from these rows' new states, in the same
-    transaction: an alert event's email outcome (#5507)."""
+    """Record on each alert event its email's outcome, once every email
+    carrying it has settled: sent when all were, else failed naming each
+    reader it didn't reach and why. Same transaction as the settling."""
+    from steward.models.alerts import AlertEvent
+    done = [r.id for r in rows if r.kind == "alert" and r.state in ("sent", "failed")]
+    if not done:
+        return
+    link = email_outbox_events.c
+    touched = select(link.event_id).where(link.outbox_id.in_(done))
+    emails: dict[str, list] = {}
+    for event_id, state, recipient, error in (await db.execute(
+            select(link.event_id, EmailOutbox.state, EmailOutbox.recipient, EmailOutbox.last_error)
+            .join(EmailOutbox, EmailOutbox.id == link.outbox_id)
+            .where(link.event_id.in_(touched)))).all():
+        emails.setdefault(event_id, []).append((state, recipient, error))
+    settled = {eid: es for eid, es in emails.items()
+               if all(state in ("sent", "failed") for state, _, _ in es)}
+    if not settled:
+        return
+    for event in (await db.execute(
+            select(AlertEvent).where(AlertEvent.id.in_(settled)))).scalars():
+        failed = sorted(f"{rcpt}: {err}" for state, rcpt, err in settled[event.id] if state == "failed")
+        event.email_sent = not failed
+        event.email_error = "; ".join(failed) or None
 
 
 async def claim_due(app: "Quart", now: datetime, limit: int = BATCH) -> list[str]: