feat(alerts): dispatch after commit; maintenance mutes, not skips; actions via trigger_run (#5063)

Notifications and alert actions now start only once the evaluating
transaction commits, and never for one that rolls back. record_metrics
queues them on the session (_after_commit), and after_commit/after_rollback
listeners on the sync session spawn or drop them. Spawned tasks are held in a
module set so they cannot be garbage-collected before they run.

A maintenance window no longer skips evaluation. Alerts in its scope still
change state, but their notifications and actions are held, and each event
says so ("notification muted: maintenance window <name>"; also on no-data
and resume events). notify_after_maintenance, run every minute with the
silence check as the new core task alert_reconcile, sends one notification
for an alert still firing after its window ends and runs its action. The
event's note records the release.

Alert actions go through runner.trigger_run like every other run path, so
the playbook and the inventory must be ones the source lists (#5034 guards).
The firing event links to the run, or its note says why the action did not
start. The maintenance page and form wording now say what a window does.

Tests:
- integration: a rolled-back evaluation notifies nobody, and dispatch waits
  for the commit; maintenance mutes while the state moves and notifies once
  when the window ends; an action whose playbook is outside its source is
  refused, with the note set;
- the alert-action test uses a listed inventory name and waits, with a
  deadline, for the run the action starts;
- unit: _muted_by.

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

steward/app.py

@@ -344,9 +344,13 @@ def _register_core_tasks(app: Quart) -> None:
         from .core.cleanup import run_cleanup as _cleanup
         await _cleanup(app)
 
-    async def run_silence_check():
-        from .core.alerts import check_silence
-        await check_silence(app)
+    async def run_alert_reconcile():
+        from .core.alerts import check_silence, notify_after_maintenance
+        for step in (check_silence, notify_after_maintenance):
+            try:
+                await step(app)
+            except Exception:
+                app.logger.exception("alert_reconcile: %s failed", step.__name__)
 
     async def run_report_check():
         from .core.reports import check_and_send
@@ -372,10 +376,11 @@ def _register_core_tasks(app: Quart) -> None:
             interval_seconds=60,
             run_on_startup=True,
         ),
-        # Rules whose series has gone quiet get a visible no_data mark.
+        # Rules whose series has gone quiet get a visible no_data mark; alerts
+        # a maintenance window muted notify once it ends.
         ScheduledTask(
-            name="alert_silence_check",
-            coro_factory=run_silence_check,
+            name="alert_reconcile",
+            coro_factory=run_alert_reconcile,
             interval_seconds=60,
             run_on_startup=False,
         ),

steward/core/alerts.py

@@ -6,7 +6,7 @@ from dataclasses import dataclass
 from datetime import datetime, timedelta, timezone
 from typing import TYPE_CHECKING
 
-from sqlalchemy import select
+from sqlalchemy import event as sa_event, select
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from steward.models.alerts import (
@@ -92,6 +92,54 @@ def _in_window(w: MaintenanceWindow, source_module: str, resource_name: str) ->
     return w.scope_source == source_module and w.scope_resource in (None, resource_name)
 
 
+MUTED = "notification muted: maintenance window"
+RELEASED = "notified when the window ended"
+
+
+def _muted_by(windows, rule: AlertRule) -> str | None:
+    """The note an event carries when a maintenance window holds back its
+    notification, or None when no window covers the rule."""
+    w = next((w for w in windows if _in_window(w, rule.source_module, rule.resource_name)), None)
+    return f"{MUTED} {w.name}" if w else None
+
+
+# Notification and action tasks in flight. Held here so the event loop's weak
+# references are not the only ones (a dropped task is silently never run).
+_background: set[asyncio.Task] = set()
+
+_AFTER_COMMIT = "steward_alert_after_commit"
+
+
+def _spawn(coro) -> None:
+    task = asyncio.create_task(coro)
+    _background.add(task)
+    task.add_done_callback(_background.discard)
+
+
+def _after_commit(session: AsyncSession, fn, *args) -> None:
+    """Run fn(app, *args) once the session's transaction commits, and never if
+    it rolls back: a notification or a playbook run must describe what was
+    actually stored."""
+    if _app is None:
+        return
+    info = session.info
+    if _AFTER_COMMIT not in info:
+        info[_AFTER_COMMIT] = []
+        app = _app
+
+        def _committed(_sync_session) -> None:
+            jobs, info[_AFTER_COMMIT] = info[_AFTER_COMMIT], []
+            for job_fn, job_args in jobs:
+                _spawn(job_fn(app, *job_args))
+
+        def _rolled_back(_sync_session) -> None:
+            info[_AFTER_COMMIT] = []
+
+        sa_event.listen(session.sync_session, "after_commit", _committed)
+        sa_event.listen(session.sync_session, "after_rollback", _rolled_back)
+    info[_AFTER_COMMIT].append((fn, args))
+
+
 async def record_metrics(session: AsyncSession, rows: list[PluginMetric]) -> None:
     """Write metric rows and evaluate the alert rules that watch them.
 
@@ -103,8 +151,9 @@ async def record_metrics(session: AsyncSession, rows: list[PluginMetric]) -> Non
     Maintenance windows, rules and their states are read once per batch, not
     once per row. Each rule sees its key's samples oldest first, so a backlog
     (an agent catching up after an outage) counts consecutive checks as they
-    happened. Notification I/O is deferred via asyncio.create_task() so no
-    network calls occur while the transaction is open.
+    happened. A maintenance window holds back notifications and actions, not
+    evaluation: the state still moves, and the event says it was muted.
+    Notifications and actions start only once the transaction commits.
     """
     if not rows:
         return
@@ -147,25 +196,23 @@ async def record_metrics(session: AsyncSession, rows: list[PluginMetric]) -> Non
             continue
         samples = sorted(by_key[(rule.source_module, rule.resource_name, rule.metric_name)],
                          key=lambda r: r.recorded_at)
-        muted = any(_in_window(w, rule.source_module, rule.resource_name) for w in windows)
-        resumed = _track_samples(session, rule, alert_state, samples, now)
-        if resumed and rule.notify_no_data and not muted and _app is not None:
+        muted = _muted_by(windows, rule)
+        resumed = _track_samples(session, rule, alert_state, samples, now, muted)
+        if resumed and rule.notify_no_data and not muted:
             event_id, value, note = resumed
-            asyncio.create_task(
-                _dispatch_notification(_app, "data_resumed", rule, value, event_id, note))
-        if muted:
-            continue  # suppressed by maintenance window
+            _after_commit(session, _dispatch_notification, "data_resumed", rule, value,
+                          event_id, note)
         for sample in samples:
             notifications = await _evaluate_rule(session, rule, alert_state, sample.value,
-                                                 sample.recorded_at)
+                                                 sample.recorded_at, note=muted)
+            if muted:
+                continue
             for notif_type, event_id in notifications:
-                if _app is not None:
-                    asyncio.create_task(
-                        _dispatch_notification(_app, notif_type, rule, sample.value, event_id))
-                    # Admin-configured Ansible action runs only on transition-to-firing.
-                    if notif_type == "firing" and rule.ansible_action:
-                        asyncio.create_task(
-                            _run_ansible_action(_app, rule, sample.value, event_id))
+                _after_commit(session, _dispatch_notification, notif_type, rule, sample.value,
+                              event_id)
+                # Admin-configured Ansible action runs only on transition-to-firing.
+                if notif_type == "firing" and rule.ansible_action:
+                    _after_commit(session, _run_ansible_action, rule, sample.value, event_id)
 
 
 # A series is silent once it has gone this long without a sample, or three of
@@ -219,11 +266,12 @@ def _duration(seconds: float) -> str:
 
 
 def _track_samples(session: AsyncSession, rule: AlertRule, state: AlertState,
-                   samples: list[PluginMetric], now: datetime
+                   samples: list[PluginMetric], now: datetime, muted: str | None = None
                    ) -> tuple[str, float, str] | None:
     """Note a rule's new samples (oldest first) for silence tracking. When the
     series had been marked no_data, clear it and record a resume event,
-    returned as (event_id, value, note) for the caller to notify."""
+    returned as (event_id, value, note) for the caller to notify. `muted` is
+    added to the event's note when a maintenance window holds it back."""
     for sample in samples:
         if state.last_sample_at is not None:
             if sample.recorded_at <= state.last_sample_at:
@@ -235,7 +283,8 @@ def _track_samples(session: AsyncSession, rule: AlertRule, state: AlertState,
         return None
     note = f"data resumed after {_duration((now - state.no_data_since).total_seconds())} silent"
     value = samples[-1].value
-    event_id = _add_event(session, rule.id, "no_data", state.state.value, value, now, note=note)
+    event_id = _add_event(session, rule.id, "no_data", state.state.value, value, now,
+                          note=f"{note}; {muted}" if muted else note)
     state.no_data_since = None
     return event_id, value, note
 
@@ -301,8 +350,10 @@ async def _evaluate_rule(
     state: AlertState,
     value: float,
     now: datetime,
+    note: str | None = None,
 ) -> list[tuple[str, str]]:
-    """Apply one sample to a rule's state. Returns [(notif_type, event_id)]."""
+    """Apply one sample to a rule's state. Returns [(notif_type, event_id)];
+    an event it records carries `note`."""
     from_state = state.state
     step = next_step(from_state, state.consecutive_failure_count, state.consecutive_ok_count,
                      _is_breached(value, rule.operator, rule.threshold),
@@ -317,7 +368,8 @@ async def _evaluate_rule(
             state.acknowledged_by = None
             state.acknowledged_at = None
     if step.event:
-        event_id = _add_event(session, rule.id, from_state.value, step.event, value, now)
+        event_id = _add_event(session, rule.id, from_state.value, step.event, value, now,
+                              note=note)
         notifications.append((step.event, event_id))
     await session.flush()
     return notifications
@@ -426,32 +478,17 @@ async def _run_ansible_action(
 ) -> None:
     """Run the rule's configured Ansible playbook (system-triggered).
 
-    Runs after the evaluation transaction commits (scheduled via create_task).
-    Creates a system-triggered AnsibleRun, links it to the firing AlertEvent,
-    then executes it with the static params plus injected alert context vars.
+    Starts after the evaluation transaction commits. Goes through
+    runner.trigger_run like every other run path, so the playbook and the
+    inventory must be ones the source lists (ansible/guards.py). The firing
+    event links to the run, or says why the action did not start.
     """
-    from steward.ansible import executor, sources as src_module
-    from steward.models.ansible import AnsibleRun, AnsibleRunStatus
+    from steward.ansible.runner import trigger_run
 
     action = rule.ansible_action or {}
-    playbook = action.get("playbook")
-    inventory = action.get("inventory")
-    if not playbook or not inventory:
-        logger.error("Ansible action for rule %r missing playbook/inventory", rule.name)
-        return
-
-    try:
-        srcs = src_module.get_sources(app.config.get("ANSIBLE", {}))
-    except Exception:
-        logger.exception("Ansible action for rule %r: could not load sources", rule.name)
-        return
-    source = next((s for s in srcs if s["name"] == action.get("source")), None)
-    if source is None:
-        logger.error("Ansible action for rule %r: source %r not configured",
-                     rule.name, action.get("source"))
-        return
-
-    # Static params + auto-injected alert context (all argv, never shell).
+    playbook = action.get("playbook") or ""
+    inventory = action.get("inventory") or ""
+    source_name = action.get("source") or ""
     params: dict = {
         "extra_vars": list(action.get("extra_vars") or []) + alert_extra_vars(rule, value),
     }
@@ -461,25 +498,32 @@ async def _run_ansible_action(
         params["tags"] = action["tags"]
     if action.get("check"):
         params["check"] = True
+    scope = inventory if inventory.startswith("steward:") else f"repo:{source_name}:{inventory}"
+
+    if not playbook or not inventory:
+        run, err = None, "the action has no playbook or inventory"
+    else:
+        try:
+            run, _source, err = await trigger_run(
+                app, source_name=source_name, playbook_path=playbook,
+                inventory_scope=scope, params=params, triggered_by=None)
+        except Exception as exc:
+            logger.exception("Ansible action for rule %r failed to start", rule.name)
+            run, err = None, str(exc)
+    if err:
+        logger.error("Ansible action for rule %r not started: %s", rule.name, err)
 
-    run_id = str(uuid.uuid4())
     async with app.db_sessionmaker() as db:
         async with db.begin():
-            db.add(AnsibleRun(
-                id=run_id,
-                playbook_path=playbook,
-                inventory_path=inventory,
-                source_name=source["name"],
-                triggered_by=None,
-                status=AnsibleRunStatus.running,
-                params=params,
-            ))
-            event = (await db.execute(
+            ev = (await db.execute(
                 select(AlertEvent).where(AlertEvent.id == event_id))).scalar_one_or_none()
-            if event:
-                event.ansible_run_id = run_id
-
-    await executor.start_run(app, run_id, playbook, inventory, source["path"], params)
+            if ev is None:
+                return
+            if run is not None:
+                ev.ansible_run_id = run.id
+            else:
+                why = f"Ansible action not started: {err}"
+                ev.note = f"{ev.note}; {why}" if ev.note else why
 
 
 async def check_silence(app: "Quart") -> None:
@@ -521,10 +565,49 @@ async def check_silence(app: "Quart") -> None:
                     note = f"no sample for {_duration((now - state.last_sample_at).total_seconds())}"
                     if state.sample_interval_s:
                         note += f" (usually every {_duration(state.sample_interval_s)})"
+                muted = _muted_by(windows, rule)
                 event_id = _add_event(session, rule.id, state.state.value, "no_data", None,
-                                      now, note=note)
-                if rule.notify_no_data and not any(
-                        _in_window(w, rule.source_module, rule.resource_name) for w in windows):
+                                      now, note=f"{note}; {muted}" if muted else note)
+                if rule.notify_no_data and not muted:
                     notify.append((rule, event_id, note))
     for rule, event_id, note in notify:
-        asyncio.create_task(_dispatch_notification(app, "no_data", rule, None, event_id, note))
+        _spawn(_dispatch_notification(app, "no_data", rule, None, event_id, note))
+
+
+async def notify_after_maintenance(app: "Quart") -> None:
+    """Notify the alerts that started firing inside a maintenance window and
+    are still firing now that no window covers them, and run their actions:
+    a window delays what it mutes, it does not discard it. Run every minute
+    by the core scheduler; each muted firing is released once."""
+    now = datetime.now(timezone.utc)
+    release: list[tuple[AlertRule, str, float | None]] = []
+    async with app.db_sessionmaker() as session:
+        async with session.begin():
+            rows = (await session.execute(
+                select(AlertState, AlertRule)
+                .join(AlertRule, AlertState.rule_id == AlertRule.id)
+                .where(AlertRule.enabled.is_(True), AlertState.state == AlertStateEnum.firing)
+            )).all()
+            if not rows:
+                return
+            windows = (await session.execute(
+                select(MaintenanceWindow).where(
+                    MaintenanceWindow.start_at <= now, MaintenanceWindow.end_at >= now)
+            )).scalars().all()
+            for state, rule in rows:
+                if _muted_by(windows, rule):
+                    continue
+                fired = (await session.execute(
+                    select(AlertEvent)
+                    .where(AlertEvent.rule_id == rule.id, AlertEvent.to_state == "firing")
+                    .order_by(AlertEvent.transitioned_at.desc()).limit(1)
+                )).scalar_one_or_none()
+                note = (fired.note or "") if fired else ""
+                if MUTED not in note or RELEASED in note:
+                    continue
+                fired.note = f"{note}; {RELEASED}"
+                release.append((rule, fired.id, fired.metric_value))
+    for rule, event_id, value in release:
+        _spawn(_dispatch_notification(app, "firing", rule, value, event_id))
+        if rule.ansible_action:
+            _spawn(_run_ansible_action(app, rule, value, event_id))

steward/templates/alerts/maintenance.html

@@ -11,8 +11,9 @@
 </div>
 
 <p style="color:var(--text-muted);font-size:0.88rem;margin-bottom:1.5rem;">
-  During an active window, alert rule evaluation is suppressed for the matching scope.
-  Metrics are still recorded — only notifications are silenced.
+  During an active window, alerts in its scope still change state and show on the Alerts page,
+  but send no notifications and run no Ansible actions. Each event a window mutes says so in
+  the alert's history. An alert that is still firing when the window ends notifies then, and runs its action.
 </p>
 
 {% if windows %}

steward/templates/alerts/maintenance_form.html

@@ -50,7 +50,7 @@
         <input type="text" name="scope_resource" id="scope-resource"
                placeholder="Exact resource name (e.g. my-server)">
         <p style="font-size:0.78rem;color:var(--text-muted);margin-top:0.25rem;">
-          Leave blank to suppress all resources within the selected source.
+          Leave blank to mute all resources within the selected source.
         </p>
       </div>
 
@@ -64,9 +64,9 @@
   <div class="card" style="margin-top:1rem;">
     <div class="section-title" style="margin-bottom:0.5rem;">How scope works</div>
     <div style="display:grid;gap:0.4rem;font-size:0.82rem;color:var(--text-muted);">
-      <div><strong style="color:var(--text);">All alerts</strong> — every alert rule is suppressed</div>
-      <div><strong style="color:var(--text);">By source</strong> — e.g. source=ping suppresses all ping alerts</div>
-      <div><strong style="color:var(--text);">By source + resource</strong> — e.g. ping/my-server suppresses only that host's ping alerts</div>
+      <div><strong style="color:var(--text);">All alerts</strong> — every alert rule's notifications are muted</div>
+      <div><strong style="color:var(--text);">By source</strong> — e.g. source=icmp mutes all ping alerts</div>
+      <div><strong style="color:var(--text);">By source + resource</strong> — e.g. icmp/my-server mutes only that host's ping alerts</div>
     </div>
   </div>
 </div>