diff --git a/docs/plans/2026-06-19-sms-notifications-design.md b/docs/plans/2026-06-19-sms-notifications-design.md new file mode 100644 index 00000000..45e1d7eb --- /dev/null +++ b/docs/plans/2026-06-19-sms-notifications-design.md @@ -0,0 +1,160 @@ +# 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 (2025–2026) 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 `` 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` 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`. +- 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).