fix(alerts): acknowledgements hold until resolve; clear after N OK; rule changes reset (#5059)

#5059 "Alert state machine: acknowledgement holds until resolve; N
consecutive OK to resolve; editing, disabling or deleting a rule resets it".

- The state machine is now next_step(), a pure function of one sample. An
  acknowledged alert stays acknowledged while breached; before, the next
  breaching sample re-fired it. Firing and acknowledged alerts resolve after
  consecutive_ok_required OK checks in a row (new rule column, default 1;
  rule form, rules list and MCP create/update_alert_rule clear_after_checks).
- Changing what a rule watches (source, resource, metric, operator,
  threshold) or disabling it resets its alert to inactive; a firing or
  acknowledged one records a resolved event whose new note says why, and
  nobody is notified. Web and MCP edits share alerts.service.apply_rule_change.
- Migration 0028: consecutive_ok_required, consecutive_ok_count,
  alert_events.note, and metric_value nullable (a reset has no sample).

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

steward/alerts/routes.py

@@ -23,6 +23,7 @@ def _rule_form(form) -> dict:
         "operator": form.get("operator", ""),
         "threshold": form.get("threshold"),
         "consecutive_failures_required": form.get("consecutive_failures_required"),
+        "consecutive_ok_required": form.get("consecutive_ok_required"),
     }
 
 
@@ -204,8 +205,7 @@ async def update_rule(rule_id: str):
                 fields = await alert_svc.rule_fields(db, **_rule_form(form))
             except ValueError as exc:
                 return str(exc), 400
-            for k, v in fields.items():
-                setattr(rule, k, v)
+            await alert_svc.apply_rule_change(db, rule, fields)
             if apply_action:
                 rule.ansible_action = ansible_action
     await log_audit(current_app, session.get("user_id"), session.get("username", ""),
@@ -222,7 +222,7 @@ async def toggle_rule(rule_id: str):
             rule = result.scalar_one_or_none()
             if rule is None:
                 return "Not found", 404
-            rule.enabled = not rule.enabled
+            await alert_svc.apply_rule_change(db, rule, {}, enabled=not rule.enabled)
     action = "alert_rule.enabled" if rule.enabled else "alert_rule.disabled"
     await log_audit(current_app, session.get("user_id"), session.get("username", ""),
                     action, entity_type="alert_rule", entity_id=rule.name)

steward/alerts/service.py

@@ -109,7 +109,8 @@ def _choices(values: list[str], limit: int = 20) -> str:
 
 
 async def rule_fields(db, *, name: str, source_module: str, resource_name: str, metric_name: str,
-                      operator: str, threshold, consecutive_failures_required=1) -> dict:
+                      operator: str, threshold, consecutive_failures_required=1,
+                      consecutive_ok_required=1) -> dict:
     """Validated column values for a new or edited alert rule.
 
     The same choices the rule form offers: a known source, a resource that
@@ -148,9 +149,39 @@ async def rule_fields(db, *, name: str, source_module: str, resource_name: str,
         raise ValueError("consecutive checks must be a whole number.") from None
     if consecutive < 1:
         raise ValueError("consecutive checks must be at least 1.")
+    try:
+        ok_checks = (1 if consecutive_ok_required in (None, "")
+                     else int(consecutive_ok_required))
+    except (TypeError, ValueError):
+        raise ValueError("checks to clear must be a whole number.") from None
+    if ok_checks < 1:
+        raise ValueError("checks to clear must be at least 1.")
     return {"name": name[:128], "source_module": source_module, "resource_name": resource_name,
             "metric_name": metric_name, "operator": op, "threshold": threshold,
-            "consecutive_failures_required": consecutive}
+            "consecutive_failures_required": consecutive,
+            "consecutive_ok_required": ok_checks}
+
+
+# Changing any of these changes what the rule is watching, so whatever state
+# it had built up no longer means anything.
+WATCHED_FIELDS = ("source_module", "resource_name", "metric_name", "operator", "threshold")
+
+
+async def apply_rule_change(db, rule: AlertRule, fields: dict, *, enabled: bool | None = None) -> None:
+    """Apply validated fields (and optionally enabled) to a rule, in the
+    caller's transaction. A change to what it watches, or disabling it, puts
+    its alert back to inactive (core.alerts.reset_rule_state)."""
+    from steward.core.alerts import reset_rule_state
+    changed = [k for k in WATCHED_FIELDS if k in fields and getattr(rule, k) != fields[k]]
+    for k, v in fields.items():
+        setattr(rule, k, v)
+    disabling = enabled is False and rule.enabled
+    if enabled is not None:
+        rule.enabled = bool(enabled)
+    if disabling:
+        await reset_rule_state(db, rule, "rule disabled")
+    elif changed:
+        await reset_rule_state(db, rule, "rule changed: " + ", ".join(changed))
 
 
 async def add_rule(db, fields: dict, *, created_by: str, enabled: bool = True,

steward/core/alerts.py

@@ -2,6 +2,7 @@ from __future__ import annotations
 import asyncio
 import logging
 import uuid
+from dataclasses import dataclass
 from datetime import datetime, timezone
 from typing import TYPE_CHECKING
 
@@ -151,6 +152,44 @@ def _is_breached(value: float, operator: AlertOperator, threshold: float) -> boo
     return False
 
 
+@dataclass(frozen=True)
+class Step:
+    """One sample's effect on an alert: the state after it, the two run
+    counters, and the event to record ("firing" / "resolved") if any."""
+    state: AlertStateEnum
+    fail_count: int
+    ok_count: int
+    event: str | None = None
+
+
+def next_step(state: AlertStateEnum, fail_count: int, ok_count: int, breached: bool,
+              fail_required: int, ok_required: int) -> Step:
+    """The alert state machine, as a pure function of one sample.
+
+    inactive → pending → firing when `fail_required` breaching checks run in a
+    row; a non-breaching check while pending goes back to inactive.
+    firing and acknowledged resolve (to inactive, with a resolved event) only
+    after `ok_required` non-breaching checks in a row; a breaching check
+    resets that run. An acknowledgement holds while the alert is still
+    breached: only a resolve clears it.
+    """
+    fail_required, ok_required = max(1, fail_required), max(1, ok_required)
+    S = AlertStateEnum
+    if state in (S.firing, S.acknowledged):
+        if breached:
+            return Step(state, fail_count + 1, 0)
+        if ok_count + 1 >= ok_required:
+            return Step(S.inactive, 0, 0, "resolved")
+        return Step(state, fail_count, ok_count + 1)
+    # inactive, pending (and the transient resolved, read as inactive)
+    if not breached:
+        return Step(S.inactive, 0, 0)
+    fails = fail_count + 1 if state == S.pending else 1
+    if fails >= fail_required:
+        return Step(S.firing, fails, 0, "firing")
+    return Step(S.pending, fails, 0)
+
+
 async def _evaluate_rule(
     session: AsyncSession,
     rule: AlertRule,
@@ -158,73 +197,56 @@ async def _evaluate_rule(
     value: float,
     now: datetime,
 ) -> list[tuple[str, str]]:
-    """Evaluate a single rule against a metric value. Returns list of (notif_type, event_id)."""
-    breached = _is_breached(value, rule.operator, rule.threshold)
+    """Apply one sample to a rule's state. Returns [(notif_type, event_id)]."""
     from_state = state.state
+    step = next_step(from_state, state.consecutive_failure_count, state.consecutive_ok_count,
+                     _is_breached(value, rule.operator, rule.threshold),
+                     rule.consecutive_failures_required, rule.consecutive_ok_required)
+    state.consecutive_failure_count = step.fail_count
+    state.consecutive_ok_count = step.ok_count
     notifications: list[tuple[str, str]] = []
-
-    if state.state == AlertStateEnum.inactive:
-        if breached:
-            state.consecutive_failure_count = 1
-            if state.consecutive_failure_count >= rule.consecutive_failures_required:
-                state.state = AlertStateEnum.firing
-                state.last_transition_at = now
-                event_id = _add_event(session, rule.id, from_state.value, "firing", value, now)
-                notifications.append(("firing", event_id))
-            else:
-                state.state = AlertStateEnum.pending
-                state.last_transition_at = now
-
-    elif state.state == AlertStateEnum.pending:
-        if breached:
-            state.consecutive_failure_count += 1
-            if state.consecutive_failure_count >= rule.consecutive_failures_required:
-                state.state = AlertStateEnum.firing
-                state.last_transition_at = now
-                event_id = _add_event(session, rule.id, from_state.value, "firing", value, now)
-                notifications.append(("firing", event_id))
-        else:
-            state.consecutive_failure_count = 0
-            state.state = AlertStateEnum.inactive
-            state.last_transition_at = now
-
-    elif state.state == AlertStateEnum.firing:
-        if not breached:
-            # RESOLVED is transient: record event, notify, then immediately go inactive
-            event_id = _add_event(session, rule.id, from_state.value, "resolved", value, now)
-            notifications.append(("resolved", event_id))
-            state.consecutive_failure_count = 0
-            state.state = AlertStateEnum.inactive
-            state.last_transition_at = now
-
-    elif state.state == AlertStateEnum.acknowledged:
-        if not breached:
-            event_id = _add_event(session, rule.id, from_state.value, "resolved", value, now)
-            notifications.append(("resolved", event_id))
-            state.consecutive_failure_count = 0
-            state.state = AlertStateEnum.inactive
-            state.last_transition_at = now
+    if step.state != from_state:
+        state.state = step.state
+        state.last_transition_at = now
+        if step.state == AlertStateEnum.inactive:
             state.acknowledged_by = None
             state.acknowledged_at = None
-        else:
-            state.consecutive_failure_count += 1
-            if state.consecutive_failure_count >= rule.consecutive_failures_required:
-                state.state = AlertStateEnum.firing
-                state.last_transition_at = now
-                event_id = _add_event(session, rule.id, from_state.value, "firing", value, now)
-                notifications.append(("firing", event_id))
-
+    if step.event:
+        event_id = _add_event(session, rule.id, from_state.value, step.event, value, now)
+        notifications.append((step.event, event_id))
     await session.flush()
     return notifications
 
 
+async def reset_rule_state(session: AsyncSession, rule: AlertRule, note: str) -> None:
+    """Put a rule's alert back to inactive because the rule itself changed
+    (what it watches, its threshold, or it was disabled). An alert that was
+    firing or acknowledged gets a resolved event carrying `note`; nobody is
+    notified, since nothing about the watched resource changed."""
+    state = (await session.execute(
+        select(AlertState).where(AlertState.rule_id == rule.id))).scalar_one_or_none()
+    if state is None:
+        return
+    now = datetime.now(timezone.utc)
+    if state.state in (AlertStateEnum.firing, AlertStateEnum.acknowledged):
+        _add_event(session, rule.id, state.state.value, "resolved", None, now, note=note)
+    if state.state != AlertStateEnum.inactive:
+        state.last_transition_at = now
+    state.state = AlertStateEnum.inactive
+    state.consecutive_failure_count = 0
+    state.consecutive_ok_count = 0
+    state.acknowledged_by = None
+    state.acknowledged_at = None
+
+
 def _add_event(
     session: AsyncSession,
     rule_id: str,
     from_state: str,
     to_state: str,
-    value: float,
+    value: float | None,
     now: datetime,
+    note: str | None = None,
 ) -> str:
     """Add an AlertEvent row and return its ID."""
     event_id = str(uuid.uuid4())
@@ -235,6 +257,7 @@ def _add_event(
         to_state=to_state,
         metric_value=value,
         transitioned_at=now,
+        note=note,
     )
     session.add(event)
     return event_id

steward/mcp/tools/alerts.py

@@ -18,8 +18,10 @@ from ._shape import (
 
 def _condition(rule: AlertRule) -> str:
     n = rule.consecutive_failures_required or 1
+    ok = rule.consecutive_ok_required or 1
     tail = f" for {n} consecutive checks" if n > 1 else ""
-    return f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}{tail}"
+    clear = f"; clears after {ok} OK checks" if ok > 1 else ""
+    return f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}{tail}{clear}"
 
 
 def _action(rule: AlertRule) -> dict | None:
@@ -104,6 +106,7 @@ async def get_alert_history(rule: str = "", range: str = "7d", limit: int = 50)
             "from_state": e.from_state,
             "to_state": e.to_state,
             "metric_value": rnd(e.metric_value),
+            "note": e.note,
             "at": iso(e.transitioned_at),
             "age_s": age_s(e.transitioned_at),
             "email_sent": e.email_sent,
@@ -226,7 +229,7 @@ def _rule_row(r: AlertRule) -> dict:
 
 async def create_alert_rule(name: str, source_module: str, resource: str, metric: str,
                             operator: str, threshold: float, consecutive_checks: int = 1,
-                            enabled: bool = True) -> dict:
+                            clear_after_checks: int = 1, enabled: bool = True) -> dict:
     """Add an alert rule: notify when `metric` of `resource` crosses `threshold`.
 
     source_module, resource, metric: get_alert_rule_options lists the valid
@@ -234,6 +237,9 @@ async def create_alert_rule(name: str, source_module: str, resource: str, metric
     operator: ">", "<", ">=", "<=", "==" or "!=".
     consecutive_checks: how many checks in a row must breach before it fires
       (default 1); higher values ride out a single blip.
+    clear_after_checks: how many checks in a row must be OK before a firing or
+      acknowledged alert resolves (default 1). An acknowledgement holds until
+      then, however long the breach lasts.
 
     Notifications go to the channels set in Steward's Settings. The playbook a
     rule runs when it fires is set separately (set_alert_action, admin).
@@ -244,7 +250,8 @@ async def create_alert_rule(name: str, source_module: str, resource: str, metric
         fields = await alert_svc.rule_fields(
             db, name=name, source_module=source_module, resource_name=resource,
             metric_name=metric, operator=operator, threshold=threshold,
-            consecutive_failures_required=consecutive_checks)
+            consecutive_failures_required=consecutive_checks,
+            consecutive_ok_required=clear_after_checks)
         r = await alert_svc.add_rule(db, fields, created_by=caller.user_id, enabled=enabled)
     await audit_mcp("alert_rule.created", "alert_rule", r.name,
                     {"rule_id": r.id, "source": r.source_module, "resource": r.resource_name,
@@ -256,12 +263,15 @@ async def update_alert_rule(rule: str, name: str | None = None, source_module: s
                             resource: str | None = None, metric: str | None = None,
                             operator: str | None = None, threshold: float | None = None,
                             consecutive_checks: int | None = None,
+                            clear_after_checks: int | None = None,
                             enabled: bool | None = None) -> dict:
     """Change an alert rule. Only the arguments you pass change; the rest stay.
 
     rule: its name or id (list_alert_rules). Pass enabled=false to silence it
     without deleting it. The changed rule is validated as create_alert_rule
-    validates a new one. Recorded in the audit log.
+    validates a new one. Changing what it watches (source, resource, metric,
+    operator, threshold) or disabling it puts its alert back to inactive; a
+    firing alert records a resolved event saying why. Recorded in the audit log.
     """
     async with db_session() as db, db.begin():
         r = await _rule(db, rule)
@@ -274,14 +284,14 @@ async def update_alert_rule(rule: str, name: str | None = None, source_module: s
             operator=r.operator.value if operator is None else operator,
             threshold=r.threshold if threshold is None else threshold,
             consecutive_failures_required=(r.consecutive_failures_required
-                                           if consecutive_checks is None else consecutive_checks))
-        for k, v in fields.items():
-            setattr(r, k, v)
-        if enabled is not None:
-            r.enabled = bool(enabled)
+                                           if consecutive_checks is None else consecutive_checks),
+            consecutive_ok_required=(r.consecutive_ok_required
+                                     if clear_after_checks is None else clear_after_checks))
+        await alert_svc.apply_rule_change(db, r, fields, enabled=enabled)
     changed = sorted(k for k, v in {"name": name, "source_module": source_module,
                                     "resource": resource, "metric": metric, "operator": operator,
                                     "threshold": threshold, "consecutive_checks": consecutive_checks,
+                                    "clear_after_checks": clear_after_checks,
                                     "enabled": enabled}.items() if v is not None)
     await audit_mcp("alert_rule.updated", "alert_rule", r.name, {"rule_id": r.id, "changed": changed})
     return _rule_row(r)

steward/migrations/versions/0028_alert_resolve_and_reset.py

@@ -0,0 +1,31 @@
+"""Alerts resolve after N OK checks; rule changes reset state with a note
+
+Revision ID: 0028_alert_resolve_and_reset
+Revises: 0027_user_auth_source
+Create Date: 2026-10-06
+"""
+from typing import Sequence, Union
+from alembic import op
+import sqlalchemy as sa
+
+revision: str = "0028_alert_resolve_and_reset"
+down_revision: Union[str, None] = "0027_user_auth_source"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    op.add_column("alert_rules", sa.Column("consecutive_ok_required", sa.Integer,
+                                           nullable=False, server_default="1"))
+    op.add_column("alert_states", sa.Column("consecutive_ok_count", sa.Integer,
+                                            nullable=False, server_default="0"))
+    op.alter_column("alert_events", "metric_value", existing_type=sa.Float, nullable=True)
+    op.add_column("alert_events", sa.Column("note", sa.Text, nullable=True))
+
+
+def downgrade() -> None:
+    op.drop_column("alert_events", "note")
+    op.execute("UPDATE alert_events SET metric_value = 0 WHERE metric_value IS NULL")
+    op.alter_column("alert_events", "metric_value", existing_type=sa.Float, nullable=False)
+    op.drop_column("alert_states", "consecutive_ok_count")
+    op.drop_column("alert_rules", "consecutive_ok_required")

steward/models/alerts.py

@@ -41,6 +41,9 @@ class AlertRule(Base):
     )
     threshold: Mapped[float] = mapped_column(Float, nullable=False)
     consecutive_failures_required: Mapped[int] = mapped_column(Integer, nullable=False, default=1)
+    # Non-breaching checks in a row before a firing (or acknowledged) alert resolves.
+    consecutive_ok_required: Mapped[int] = mapped_column(Integer, nullable=False, default=1,
+                                                         server_default="1")
     enabled: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True)
     # Optional admin-only action: run an Ansible playbook on transition-to-firing.
     # Shape: {source, playbook, inventory, extra_vars, limit, tags, check}. NULL = none.
@@ -62,6 +65,8 @@ class AlertState(Base):
         Enum(AlertStateEnum), nullable=False, default=AlertStateEnum.inactive
     )
     consecutive_failure_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0)
+    consecutive_ok_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0,
+                                                      server_default="0")
     last_transition_at: Mapped[datetime] = mapped_column(
         DateTime(timezone=True), nullable=False, default=lambda: datetime.now(timezone.utc)
     )
@@ -98,7 +103,10 @@ class AlertEvent(Base):
     )
     from_state: Mapped[str] = mapped_column(String(32), nullable=False)
     to_state: Mapped[str] = mapped_column(String(32), nullable=False)
-    metric_value: Mapped[float] = mapped_column(Float, nullable=False)
+    # None when the transition came from a change to the rule, not a sample.
+    metric_value: Mapped[float | None] = mapped_column(Float, nullable=True)
+    # Why a transition happened when the value alone doesn't say (a rule edit).
+    note: Mapped[str | None] = mapped_column(Text, nullable=True)
     transitioned_at: Mapped[datetime] = mapped_column(
         DateTime(timezone=True), nullable=False, default=lambda: datetime.now(timezone.utc)
     )

steward/templates/alerts/list.html

@@ -61,7 +61,7 @@
             <tr>
                 <th>Name</th>
                 <th>Condition</th>
-                <th style="text-align:center;">Consec.</th>
+                <th style="text-align:center;" title="Checks in a row to fire / to clear">Fire / clear</th>
                 <th></th>
             </tr>
         </thead>
@@ -80,7 +80,7 @@
                     :{{ rule.metric_name }} {{ rule.operator.value }} {{ rule.threshold }}
                   </span>
                 </td>
-                <td style="color:var(--text-muted);text-align:center;">{{ rule.consecutive_failures_required }}</td>
+                <td style="color:var(--text-muted);text-align:center;">{{ rule.consecutive_failures_required }} / {{ rule.consecutive_ok_required }}</td>
                 <td class="td-actions">
                     <a href="/alerts/rules/{{ rule.id }}/edit" class="btn btn-sm btn-ghost">Edit</a>
                     <form method="post" action="/alerts/rules/{{ rule.id }}/toggle" style="display:inline;">{{ csrf_field() }}

steward/templates/alerts/rules_form.html

@@ -90,6 +90,15 @@
                value="{{ rule.consecutive_failures_required if rule else 1 }}">
       </div>
 
+      <div class="form-group">
+        <label>Consecutive OK checks before it clears</label>
+        <input type="number" name="consecutive_ok_required" min="1"
+               value="{{ rule.consecutive_ok_required if rule else 1 }}">
+        <p style="font-size:0.75rem;color:var(--text-muted);margin-top:0.2rem;">
+          A firing or acknowledged alert resolves only after this many checks in a row are back
+          within the threshold. An acknowledgement holds until then.</p>
+      </div>
+
       {# ── Metric reference ────────────────────────────────────────────────── #}
       <details style="margin-bottom:1rem;">
         <summary style="cursor:pointer;font-size:0.82rem;color:var(--text-muted);user-select:none;">