fix: stop emailing RESOLVED after every successful sync

docs/configuration.rst

@@ -311,7 +311,11 @@ dispatched. The flag file (``ops.alert_flag``) is **always** written
 regardless of the transport setting — it serves as a simple signal for
 external monitoring tools.
 
-Optionally, an email alert can be sent via SMTP or Mailgun.
+Optionally, an email alert can be sent via SMTP or Mailgun. Every failing
+cycle sends an ``ALERT``, including consecutive failures on the same
+condition — a halt that repeats is a sync that is still not happening.
+``RESOLVED`` is sent once, on the cycle that clears an alert that was actually
+active, so a healthy daemon does not mail after every sync.
 
 Flag-file only (default)
 """"""""""""""""""""""""

src/door_sync/alert.py

@@ -5,6 +5,11 @@
 regardless of transport — external monitoring (Nagios, Prometheus
 textfile collector, etc.) can detect halts without parsing logs.
 
+Every failing cycle sends an ALERT, including consecutive failures on the same
+condition. RESOLVED is edge-triggered on the flag: it goes out once, on the
+cycle that clears an alert that was actually active, so a healthy daemon does
+not mail after every sync.
+
 Email failures are logged at ERROR but never crash a reconcile cycle.
 """
 
@@ -36,6 +41,11 @@ def raise_(
 ) -> None:
     """Write flag file and, if configured, send an alert email.
 
+    Every failing cycle mails, including consecutive failures on the same
+    condition. A halt that repeats is a sync that is still not happening, and
+    each one is worth surfacing -- only RESOLVED is edge-triggered, so that a
+    healthy daemon stays silent (see `clear()`).
+
     Args:
         reason: Human-readable description of the alert condition.
         path: Path to the alert flag file.
@@ -52,18 +62,39 @@ def clear(
     path: Path,
     alert_config: AlertConfig | None = None,
 ) -> None:
-    """Remove flag file and, if configured, send a resolved email.
+    """Remove flag file and, if an alert was active, send a resolved email.
+
+    RESOLVED is edge-triggered on the flag file: it goes out only on the cycle
+    that actually removes a flag, so a healthy daemon does not mail the
+    operator once per reconcile. A cycle that finds no flag has nothing to
+    resolve and says nothing.
+
+    If the flag could not be written when the alert was raised, no RESOLVED
+    follows that alert -- the unwritable path is already on the logger.
+
+    A RESOLVED that fails to send is not retried. The flag is already gone by
+    then, and that -- not the email -- is what external monitoring reads, so
+    the recovery is not actually lost. Re-raising the flag to force a retry
+    would tell monitoring the system is still halted, which is worse than a
+    missed courtesy email. The send failure is logged at ERROR with the
+    subject, so a dropped notification is traceable.
 
     Args:
         path: Path to the alert flag file.
         alert_config: Email transport settings, or None for flag-file only.
     """
     try:
-        path.unlink(missing_ok=True)
+        path.unlink()
+    except FileNotFoundError:
+        # No alert was active. Nothing to clear, nobody to notify.
+        return
     except OSError as exc:
         # A flag that cannot be cleared errs toward alarming, which is the safe
-        # direction, but the operator needs to know why it is stuck.
+        # direction, but the operator needs to know why it is stuck. No RESOLVED
+        # either: the flag is still there, so this was not a transition, and a
+        # stuck flag would otherwise mail on every subsequent cycle.
         _logger.error("could not clear alert flag %s: %s", path, exc)
+        return
     if alert_config is not None:
         _dispatch(alert_config, subject="RESOLVED", body="Previous alert cleared.")
 
@@ -117,7 +148,7 @@ def _send_smtp(cfg: SmtpConfig, *, subject: str, body: str) -> None:
             server.send_message(msg)
         _logger.info("alert email sent via SMTP: %s", subject)
     except Exception as exc:
-        _logger.error("failed to send alert email via SMTP", exc_info=exc)
+        _logger.error("failed to send alert email via SMTP: %s", subject, exc_info=exc)
 
 
 def _send_mailgun(cfg: MailgunConfig, *, subject: str, body: str) -> None:
@@ -138,4 +169,4 @@ def _send_mailgun(cfg: MailgunConfig, *, subject: str, body: str) -> None:
         resp.raise_for_status()
         _logger.info("alert email sent via Mailgun: %s", subject)
     except Exception as exc:
-        _logger.error("failed to send alert email via Mailgun", exc_info=exc)
+        _logger.error("failed to send alert email via Mailgun: %s", subject, exc_info=exc)

tests/test_alert.py

@@ -317,3 +317,120 @@ def test_clear_survives_unremovable_flag(tmp_path: Path, caplog: pytest.LogCaptu
         holder.chmod(0o700)
 
     assert "could not clear alert flag" in caplog.text
+
+
+# --- RESOLVED is edge-triggered on the flag, not sent every cycle ---
+
+
+def test_clear_without_active_flag_sends_no_email(tmp_path: Path) -> None:
+    """A healthy cycle must stay silent.
+
+    clear() runs after every successful reconcile, so dispatching RESOLVED
+    unconditionally mails the operator once per cycle forever.
+    """
+    path = tmp_path / "alert.flag"
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+
+    with patch("door_sync.alert.httpx.post") as mock_post:
+        alert.clear(path=path, alert_config=cfg)
+
+    mock_post.assert_not_called()
+    assert not path.exists()
+
+
+def test_clear_sends_resolved_once_then_stays_silent(tmp_path: Path) -> None:
+    path = tmp_path / "alert.flag"
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+
+    with patch("door_sync.alert.httpx.post") as mock_post:
+        alert.raise_("safety halt", path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+
+    subjects = [c.kwargs["data"]["subject"] for c in mock_post.call_args_list]
+    assert subjects == ["[door-sync] ALERT", "[door-sync] RESOLVED"]
+
+
+@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file permissions")
+def test_clear_sends_no_resolved_when_flag_cannot_be_removed(tmp_path: Path) -> None:
+    """A stuck flag is not a transition, so it must not mail on every cycle."""
+    holder = tmp_path / "ro"
+    holder.mkdir()
+    flag = holder / "alert.flag"
+    flag.write_text("stale\n")
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+    holder.chmod(0o500)
+    try:
+        with patch("door_sync.alert.httpx.post") as mock_post:
+            alert.clear(path=flag, alert_config=cfg)
+    finally:
+        holder.chmod(0o700)
+
+    mock_post.assert_not_called()
+
+
+# --- ALERT is per-failure, not per-episode ---
+
+
+def test_raise_mails_every_failing_cycle(tmp_path: Path) -> None:
+    """Consecutive failures each mail: a repeat halt is a sync still not happening."""
+    path = tmp_path / "alert.flag"
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+
+    with patch("door_sync.alert.httpx.post") as mock_post:
+        alert.raise_("mass deactivation: 12 of 30 active users", path=path, alert_config=cfg)
+        alert.raise_("mass deactivation: 12 of 30 active users", path=path, alert_config=cfg)
+        alert.raise_(
+            "crashed: ConnectionError: controller unreachable", path=path, alert_config=cfg
+        )
+
+    assert mock_post.call_count == 3
+    bodies = [c.kwargs["data"]["text"] for c in mock_post.call_args_list]
+    assert bodies[1] == "mass deactivation: 12 of 30 active users"
+    assert bodies[2] == "crashed: ConnectionError: controller unreachable"
+    # The flag tracks the most recent reason.
+    assert path.read_text(encoding="utf-8") == "crashed: ConnectionError: controller unreachable\n"
+
+
+def test_repeated_failures_then_recovery_sends_one_resolved(tmp_path: Path) -> None:
+    path = tmp_path / "alert.flag"
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+
+    with patch("door_sync.alert.httpx.post") as mock_post:
+        alert.raise_("safety halt", path=path, alert_config=cfg)
+        alert.raise_("safety halt", path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+        alert.clear(path=path, alert_config=cfg)
+
+    subjects = [c.kwargs["data"]["subject"] for c in mock_post.call_args_list]
+    assert subjects == [
+        "[door-sync] ALERT",
+        "[door-sync] ALERT",
+        "[door-sync] RESOLVED",
+    ]
+
+
+def test_failed_resolved_dispatch_names_the_lost_notification(
+    tmp_path: Path,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A RESOLVED that cannot be delivered is not retried, so the log must name it.
+
+    The flag is already gone, so external monitoring sees the recovery either
+    way -- but the operator waiting on the all-clear email needs to be able to
+    tell from the log which notification was dropped.
+    """
+    path = tmp_path / "alert.flag"
+    path.write_text("reason\n", encoding="utf-8")
+    cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config())
+
+    with (
+        patch("door_sync.alert.httpx.post", side_effect=ConnectionError("refused")),
+        caplog.at_level(logging.ERROR, logger="door_sync.alert"),
+    ):
+        alert.clear(path=path, alert_config=cfg)
+
+    assert not path.exists()
+    assert any("RESOLVED" in r.getMessage() for r in caplog.records)