From 6b3a09ec1c2ff3227fc9aa186b2d0f171fcfc35d Mon Sep 17 00:00:00 2001 From: Nirav Patel Date: Tue, 18 Aug 2026 20:50:37 -0400 Subject: [PATCH] Say when a channel cannot be checked, instead of skipping it The estate's SendGrid key is send-only: mail.send and the batch scopes, with no suppression.read, no bounces.read, no stats. So email delivery cannot be reconciled the way SMS now is. The tempting move is to check SMS and quietly say nothing about email. That is precisely how a gap disappears - the daily report looks clean and the unchecked channel stops being a question anyone asks. It now reports 'email delivery NOT VERIFIED' with the exact scope that would fix it, every day, until somebody does. Co-Authored-By: Claude Opus 5 (1M context) --- app/reconcile.py | 70 +++++++++++++++++++++++++++++++++++++++++ scripts/reconcile.py | 13 ++++++-- tests/test_reconcile.py | 37 ++++++++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/app/reconcile.py b/app/reconcile.py index 08bc60e..ed8e713 100644 --- a/app/reconcile.py +++ b/app/reconcile.py @@ -169,3 +169,73 @@ def _get(url: str, *, headers: dict[str, str], timeout: float) -> Response: def window_start(hours: int) -> datetime: return datetime.now(timezone.utc) - timedelta(hours=hours) + + +# --------------------------------------------------------------- email + +SUPPRESSION_URL = "https://api.sendgrid.com/v3/suppression/bounces" + +# The scope a key needs before any of this can be answered. Named here so the +# daily report can say exactly what to change rather than "not available". +EMAIL_READ_SCOPE = "suppression.read" + + +@dataclass(frozen=True) +class EmailReport: + checked: bool + bounces: list[dict[str, Any]] + reason: str = "" + + def summary(self) -> str: + if not self.checked: + return f"email delivery NOT VERIFIED — {self.reason}" + if not self.bounces: + return "email: no bounces in the window" + return f"email: {len(self.bounces)} bounced" + + +def email_bounces( + *, + api_key: str, + since: datetime, + timeout: float = 15.0, + get: Callable[..., Response] | None = None, +) -> EmailReport: + """Bounces SendGrid recorded, if this key is allowed to ask. + + The estate's key is send-only, so this normally reports that it could not + check. That is the point: an unanswerable question should appear in the + daily report as unanswered, not be quietly skipped. A silent skip is how + "we have monitoring" turns into seven months of unnoticed failures. + """ + if not api_key: + return EmailReport(checked=False, bounces=[], reason="no SendGrid key configured") + + fetcher = get or _get + reply = fetcher( + f"{SUPPRESSION_URL}?start_time={int(since.timestamp())}", + headers={"Authorization": f"Bearer {api_key}"}, + timeout=timeout, + ) + if reply.status in (401, 403): + return EmailReport( + checked=False, bounces=[], + reason=(f"the SendGrid key cannot read bounces (needs {EMAIL_READ_SCOPE}); " + "it is send-only, so email failures are invisible here")) + if reply.status >= 400: + return EmailReport(checked=False, bounces=[], reason=f"SendGrid answered {reply.status}") + + try: + rows = json.loads(reply.body or "[]") + except json.JSONDecodeError: + return EmailReport(checked=False, bounces=[], reason="SendGrid did not return JSON") + return EmailReport(checked=True, bounces=rows if isinstance(rows, list) else []) + + +def describe_email(report: EmailReport) -> str: + lines = [report.summary()] + for row in report.bounces[:10]: + address = str(row.get("email") or "") + masked = ("***@" + address.split("@")[-1]) if "@" in address else "***" + lines.append(f" {masked}: {str(row.get('reason') or '')[:90]}") + return "\n".join(lines) diff --git a/scripts/reconcile.py b/scripts/reconcile.py index 8732ba3..5634c97 100644 --- a/scripts/reconcile.py +++ b/scripts/reconcile.py @@ -24,7 +24,9 @@ import sys sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from app.config import Settings # noqa: E402 -from app.reconcile import describe, fetch, reconcile, window_start # noqa: E402 +from app.reconcile import ( # noqa: E402 + describe, describe_email, email_bounces, fetch, reconcile, window_start, +) def main() -> int: @@ -45,7 +47,14 @@ def main() -> int: report = reconcile(messages) text = describe(report) - if report.ok and args.quiet_when_clean: + # Email is asked about too, and says plainly when it cannot be answered — + # a check that silently skips the channel it cannot see is how a gap + # becomes invisible. + email = email_bounces(api_key=settings.sendgrid_key, + since=window_start(args.hours), timeout=settings.timeout) + text = text + "\n\n" + describe_email(email) + + if report.ok and email.checked and not email.bounces and args.quiet_when_clean: return 0 print(text) diff --git a/tests/test_reconcile.py b/tests/test_reconcile.py index 528a1a6..e7271b2 100644 --- a/tests/test_reconcile.py +++ b/tests/test_reconcile.py @@ -99,3 +99,40 @@ def test_a_refused_ledger_query_is_loud(): def test_window_start_looks_backwards(): assert window_start(24) < datetime.now(timezone.utc) + + +# --------------------------------------------------------------- email + +from app.reconcile import EmailReport, describe_email, email_bounces # noqa: E402 + + +def test_a_key_that_cannot_read_bounces_says_so_instead_of_reporting_clean(): + """The estate's key is send-only. Skipping the check quietly is how a + channel nobody can see becomes a channel nobody checks.""" + def forbidden(url, *, headers, timeout): + return Response(status=403, body='{"errors":[{"message":"access forbidden"}]}', headers={}) + + report = email_bounces(api_key="SG.x", since=window_start(24), get=forbidden) + + assert report.checked is False + assert report.bounces == [] + assert "suppression.read" in report.reason + assert "NOT VERIFIED" in describe_email(report) + + +def test_bounces_are_reported_without_publishing_addresses(): + def ledger_of_bounces(url, *, headers, timeout): + return Response(status=200, headers={}, body=json.dumps([ + {"email": "someone@example.test", "reason": "550 5.1.1 user unknown"}])) + + report = email_bounces(api_key="SG.x", since=window_start(24), get=ledger_of_bounces) + text = describe_email(report) + + assert report.checked and len(report.bounces) == 1 + assert "***@example.test" in text + assert "someone@example.test" not in text + + +def test_no_key_is_reported_as_unverified_not_as_healthy(): + report = email_bounces(api_key="", since=window_start(24)) + assert report.checked is False and "no SendGrid key" in report.reason