Files
ScadaBridge/docs/plans/2026-06-19-sms-notifications-design.md
T

161 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# SMS Notifications (T9 pivot + T10) — Design
**Date:** 2026-06-19
**Status:** Approved (full scope) — proceed to implementation plan.
**Supersedes:** the T9 "Teams delivery adapter" direction. T9/T10 were deferred to "next major version" in M6 (`docs/plans/2026-06-17-m6-kpi-history-design.md`). This design pulls them forward and **pivots the second delivery channel from Microsoft Teams to SMS (Twilio)**.
---
## Goal
Add `NotificationType.Sms` and a Twilio-REST `SmsNotificationDeliveryAdapter` behind the existing `INotificationDeliveryAdapter` seam, plus the T10 enum/UI/CLI plumbing, so notification lists can deliver to ZB team members by text message. Email delivery is unchanged.
## Why the pivot (Teams → SMS)
The user's requirement is "notify ZB team members individually." Investigation of the current Microsoft Graph / Teams platform (20252026) established a hard architectural constraint:
- An **outbound-only** backend (our on-prem central cluster, which already does OAuth2 client-credentials to `login.microsoftonline.com` for SMTP) **cannot** send a full 1:1 chat-message body to a person via Graph. App-only chat sending is migration-only; real-time chat sending is delegated-user-only.
- A full-body per-person Teams DM requires a **Bot Framework bot with a publicly-reachable *inbound* HTTPS endpoint** that Azure Bot Service calls into the datacenter — a significant security/deployment change for an on-prem SCADA system.
- The only purely-outbound per-person Teams mechanism (`sendActivityNotification`) delivers a ~150-char headline + deep link, not a message body, and requires a published Teams app installed per recipient.
SMS sidesteps all of that: it is inherently per-person (recipient = phone number), carries a full message body, and is **outbound-only** (a single HTTPS POST to the provider). It is the natural "second delivery type" that makes the T10 Type selector meaningful.
**Provider decision:** Twilio, called directly over its REST API (no SDK), honoring the project's no-new-NuGet-package rule (central package management). `Microsoft.Extensions.Http` is already in central package management (used by NotificationService/ExternalSystemGateway); the NotificationOutbox project gains a `<PackageReference>` to it — no new package version is introduced.
## Architecture
Mirror the existing Email/SMTP stack:
- A notification list's existing `NotificationList.Type` field decides the channel.
- `Notify.To("list")` / `Notify.Send` stay type-agnostic at the site.
- Central resolves the **list's Type at ingest** and stamps it on the `Notification` row; the outbox actor already dispatches via `Dictionary<NotificationType, INotificationDeliveryAdapter>` and parks on a missing adapter, so routing is automatic.
## Tech stack
C#/.NET 10, Akka.NET, EF Core (MS SQL central), Blazor Server (Central UI), System.CommandLine (CLI), `IHttpClientFactory`, ASP.NET Data Protection (`EncryptedStringConverter`) for secret-at-rest. No new NuGet packages.
---
## Decisions
### D1 — Delivery adapter (Twilio REST)
- New `SmsNotificationDeliveryAdapter : INotificationDeliveryAdapter` (`Type => NotificationType.Sms`) in `NotificationOutbox/Delivery`, registered alongside the Email adapter in `ServiceCollectionExtensions`.
- Request: `POST {ApiBaseUrl}/2010-04-01/Accounts/{AccountSid}/Messages.json`, HTTP Basic auth (`AccountSid:AuthToken`), `application/x-www-form-urlencoded` body `To` / `From` (or `MessagingServiceSid`) / `Body`. Named `HttpClient` `"Twilio"` via `IHttpClientFactory`.
- Success = Twilio 2xx (message accepted/queued). Note: true *delivery* confirmation needs a status-callback webhook (inbound) — out of scope; "accepted by Twilio" is treated as Delivered, exactly as the Email adapter treats "accepted by the SMTP server."
### D2 — Secret-at-rest (`SmsConfiguration` entity)
- New `SmsConfiguration` entity, provider-neutral name, mirroring `SmtpConfiguration`:
- `Id`, `AccountSid` (plaintext — also appears in the URL path), `AuthToken` (**encrypted** via `EncryptedStringConverter`), `FromNumber` (E.164) and/or optional `MessagingServiceSid`, optional `ApiBaseUrl` (default `https://api.twilio.com`; lets tests point at a fake handler / supports regional), `ConnectionTimeoutSeconds`, `MaxRetries`, `RetryDelay`.
- EF: add `AuthToken` to `ScadaBridgeDbContext.ApplySecretColumnEncryption`. Idempotent migration adds the table.
- `CredentialRedactor.Scrub(message, authToken)` removes the AuthToken from any logged error.
### D3 — Recipient model (per-list typing)
- A list is entirely Email **or** entirely SMS (single `NotificationList.Type`).
- `NotificationRecipient` gains a nullable `PhoneNumber` (E.164, e.g. nvarchar(32)); `EmailAddress` is relaxed to **nullable** (currently NOT NULL). Email-list recipients carry `EmailAddress`; SMS-list recipients carry `PhoneNumber`.
- Type-aware validation: SMS recipients require a valid E.164 `PhoneNumber`; Email recipients require a valid `EmailAddress`.
- Constructor/factory: keep `NotificationRecipient(name, email)`; add an SMS construction path (factory `NotificationRecipient.ForSms(name, phone)` / `ForEmail(name, email)` or object-initializer support).
- Migration: additive nullable `PhoneNumber` column + one NOT-NULL→NULL relaxation on `EmailAddress`. Idempotent.
### D4 — Message format
- SMS has no subject. Body = `Subject` + newline + `Body` (whichever present), plain text, truncated to a configurable cap (`SmsOptions.MaxMessageLength`, default 1600 = Twilio max) with an ellipsis when over.
- Doc caveat: Twilio segments at 160 GSM-7 chars and bills per segment.
### D5 — Error classification
- Small local `SmsErrorClassifier` in `NotificationOutbox/Delivery`, mirroring the pattern of `SmtpErrorClassifier` (no cross-project reference to ExternalSystemGateway):
- **Transient:** HTTP 5xx, 408, 429; `HttpRequestException`, `TaskCanceledException`/timeout (non-caller-cancel).
- **Permanent:** other 4xx (incl. 401/403 bad creds, 400 invalid/unsubscribed number).
### D6 — Per-recipient rollup (no SMS BCC → one POST per number)
- Send one Twilio request per recipient; classify each.
- **All accepted →** `Success` (`ResolvedTargets` = the numbers).
- **Any transient →** `Transient` (the whole notification retries at the fixed interval, then Parks after max-retries). *Accepted caveat:* numbers already accepted on a prior attempt get re-texted on retry — the same "re-send to all" characteristic the Email adapter already has with BCC. (v1 does NOT track per-recipient state; that is a documented future enhancement.)
- **No transient, mix of accepted + permanent-bad →** `Success` to the good numbers; permanently-bad numbers recorded in `LastError`. Do not park if anything got through.
- **All permanent / no recipients / no SMS config / list-not-found →** `Permanent` (Park).
### D7 — Ingest type-stamping
- At `NotificationOutboxActor` ingest (currently hard-codes `NotificationType.Email`, ~line 1147), resolve the target list by `ListName` and stamp the Notification's `Type` from the **list's** Type (default Email if the list is not found — delivery parks on "list not found" regardless). Keeps `Notify.To` type-agnostic.
### D8 — T10 UI/CLI plumbing
- `NotificationType.Sms` added to the enum.
- `Create/UpdateNotificationListCommand` gain `NotificationType Type` (additive, default Email); handlers persist it.
- CLI: `notification list create/update --type email|sms`, with `--emails` (email lists) / `--phones` (sms lists).
- Central UI `NotificationListForm`: an **adapter-gated** Type selector — it offers only `NotificationType` values that have a registered `INotificationDeliveryAdapter` (Email + SMS today; auto-extends if more are added). The recipient input switches email↔phone by the selected type with appropriate validation.
- Type is chosen at **create** and is **fixed on edit** (prevents email/phone recipient mismatch within a list).
- `NotificationLists` list page gains a **Type** column.
- New SMS-config management mirroring SMTP: `ListSmsConfigsCommand` / `UpdateSmsConfigCommand` (+ `HandleListSmsConfigs` / `HandleUpdateSmsConfig`, role-gated like SMTP, audited via a credential-free public shape), CLI `notification sms list|update`, Admin-only Central UI page `/notifications/sms`.
### D9 — Transport
- `NotificationRecipientDto` gains a nullable `PhoneNumber` (backward-compatible: omitted = null).
- New `SmsConfigDto` (secret rides the existing `SecretsBlock`), mirroring `SmtpConfigDto`, carried in `BundleContentDto` / `EntityAggregate` and round-tripped by `EntitySerializer`.
---
## Blast radius (file-level)
**Commons**
- `Types/Enums/NotificationType.cs` — add `Sms`.
- `Entities/Notifications/NotificationRecipient.cs` — add `PhoneNumber`; SMS construction path.
- `Entities/Notifications/SmsConfiguration.cs`**new**.
- `Messages/Management/NotificationCommands.cs` — add `Type` to Create/Update list commands; add `ListSmsConfigsCommand` / `UpdateSmsConfigCommand`.
- `Interfaces/Repositories/INotificationRepository.cs` — add SMS-config read/write methods (mirror SMTP).
**Configuration Database**
- `Configurations/NotificationConfiguration.cs``EmailAddress` nullable, `PhoneNumber` mapping; `SmsConfiguration` mapping.
- `ScadaBridgeDbContext.cs``ApplySecretColumnEncryption` adds `SmsConfiguration.AuthToken`; `DbSet<SmsConfiguration>`.
- Repository impl — SMS-config methods.
- New idempotent EF migration (PhoneNumber + EmailAddress-nullable + SmsConfiguration table).
**NotificationOutbox**
- `Delivery/SmsNotificationDeliveryAdapter.cs`**new**.
- `Delivery/SmsErrorClassifier.cs`**new**.
- `ServiceCollectionExtensions.cs` — register SMS adapter + named `"Twilio"` HttpClient.
- `NotificationOutboxActor.cs` — ingest type-stamping (D7).
- `*.csproj` — add `Microsoft.Extensions.Http` PackageReference (already centrally managed).
- `SmsOptions` (`ScadaBridge:Sms`) — timeout / max-length (retry reuses outbox settings where possible).
**Management Service**
- `ManagementActor.cs``HandleListSmsConfigs` / `HandleUpdateSmsConfig` (+ role gating + audit public shape); list-command handlers persist `Type`.
**CLI**
- `Commands/NotificationCommands.cs``--type` + `--phones` on list create/update; `notification sms list|update` group.
- CLI `README.md`.
**Central UI**
- `Components/Pages/Notifications/NotificationListForm.razor` — Type selector (adapter-gated) + per-type recipient input.
- `Components/Pages/Notifications/NotificationLists.razor` — Type column.
- `Components/Pages/Notifications/SmsConfiguration.razor`**new** (`/notifications/sms`, mirror SMTP page).
- Nav entry for the SMS config page.
**Transport**
- `Serialization/EntityDtos.cs``PhoneNumber` on recipient DTO; `SmsConfigDto`.
- `Serialization/EntitySerializer.cs` — map SMS config + recipient phone both directions.
- `BundleContentDto` / `EntityAggregate` — include SMS configs.
**Tests** (break / add)
- **Update** `NotificationOutbox.Tests/ServiceRegistrationTests.cs``Assert.Single(adapters)` → assert Email **and** SMS registered.
- New `SmsNotificationDeliveryAdapter` unit tests (fake `HttpMessageHandler`: 201 / 5xx / 429 / timeout / 400 / 401 / rollup / redaction).
- Ingest type-stamping test.
- CLI tests (`--type`, `--phones`, `sms` group).
- bUnit: Type selector (adapter-gated), per-type recipient editor, Type column.
- EF/migration config tests.
- Playwright: SMS-list create flow (INT).
**Docs / deploy**
- `docs/requirements/Component-NotificationService.md`, `Component-NotificationOutbox.md`.
- `CLAUDE.md` decision notes (NotificationType = Email+Sms; **T9/T10 delivered as SMS, not deferred**; flip the M6 deferral note).
- `README.md` component table / CLI README as needed.
- Docker: no new secret (SMS config lives in DB like SMTP); minimal `ScadaBridge:Sms` options.
---
## Out of scope / deferred (logged, not built)
- Per-recipient delivery-state tracking to avoid duplicate texts on retry (v1 uses unit-retry, matching Email-BCC; see D6).
- Twilio status-callback (inbound) webhook for true delivery confirmation.
- Microsoft Teams delivery (dropped).
- Mixed email+phone single list (per-list typing only).
- Editable list Type after creation.
## Execution
- Dedicated worktree `feat/sms-notifications` off `main` (`c72d7b79`); pathspec commits (`-m` before `--`, never `git add -A`); targeted builds/tests per task; full-solution build + docker rebuild + Playwright + live smoke at INT; then `finishing-a-development-branch` (merge-to-local-main per established pattern — push only on explicit confirmation).
- Plan + `.tasks.json` co-located in `docs/plans/`. Subagent-driven execution (classification-driven review chains).