diff --git a/README.md b/README.md index 9f95f88..699ceab 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ sidecar as a service, and hands you the values the website needs. | [BACKEND_DESIGN.md](website/BACKEND_DESIGN.md) | API contract, DB schema, security model | | [ARCHITECTURE.md](website/ARCHITECTURE.md) | The system diagram — how core, an installed module, the sidecar and the clients fit together | | [TEAMS.md](website/TEAMS.md) | Teams as a platform primitive: roster, forums, notifications, Discord slash commands and voice — design of record | +| [ENGAGEMENT.md](website/ENGAGEMENT.md) | The engagement system: module-declared event triggers, rules, cooldowns, templates and the email / push / in-app delivery channels — design of record | | [HERO_EDITOR.md](website/HERO_EDITOR.md) | Hero canvas editor feature spec | | [THEMING_AND_NAV.md](website/THEMING_AND_NAV.md) | Admin-configurable theme, brand assets and navigation — build contract | | [MODULE_SYSTEM.md](website/MODULE_SYSTEM.md) | Making the site game-agnostic: game logic becomes an installable module — design of record | diff --git a/website/ENGAGEMENT.md b/website/ENGAGEMENT.md new file mode 100644 index 0000000..bb71204 --- /dev/null +++ b/website/ENGAGEMENT.md @@ -0,0 +1,1503 @@ +# The Engagement System — findings and plan + +**Status:** design of record for the next workstream. No code written yet. The five scope decisions below +are settled; the seven questions in §7.1 are open and none of them block Phase 1 or Phase 2. Per CLAUDE.md +§ Conventions, no implementation starts without the org lead's approval of the phase it belongs to. + +**Scope decisions, settled by the org lead (2026-08-28):** + +1. **The in-app channel is in scope.** It does not exist today and has to be built, not adapted. +2. **The Teams notification pipeline is generalized and migrated onto the new system**, not built beside it. +3. **The `house.decay` protocol enrichment is in scope**, as a coordinated four-repo `PROTOCOL_VERSION` bump. +4. **Gmail OAuth2 is removed, not retained as a transport.** SMTP is the baseline; the OAuth2 consent + flow, its two routes, its borrowed Google client and its stored refresh token all go. See §1.2a for + what that deletes and §6/Phase 1 for the operator cutover it forces. +5. **The system ships with a seeded set of working templates and an editor**, so a fresh deployment + sends correctly-branded mail before anyone opens the editor. See §4.6. + +--- + +## Part 0 — Five findings that contradict the brief + +Stated up front because the rest of the document is shaped by them. + +### 0.1 There is no In-app channel. The target diagram's "existing In-app / Push" is one channel, not two + +Core has exactly one notification sink: **content-free push tickles** to ntfy/UnifiedPush +(`website/server/src/utils/pushDispatch.js`). There is no notification table, no read API, no +mark-read, and no in-app list on either client. `android-app/.../ui/notifications/NotificationsScreen.kt` +is a *preferences* screen. `module-uo`'s SSE stream (`server/utils/shardBroadcast.js`) is a live game +feed, not an inbox. + +So "add Email as a third channel" is really **add two channels to a system that has one**, and the +in-app one needs its own storage, read API and two client surfaces. + +### 0.2 Email is already two-thirds of an engagement system — scoped to Teams + +The brief frames the current email implementation as "the Gmail OAuth2 code". Gmail OAuth2 is only the +*transport*. Sitting on top of it, `utils/teamNotify.js` + `utils/teamDigestWorker.js` + +`team_notification_prefs` already implement: + +| Engagement concern | Where it already lives | +| --- | --- | +| Trigger → recipient selection | `teamNotify.js:recipientIds` / `emailRecipients` via the Team access resolver | +| Per-user opt-in, per-scope | `team_notification_prefs.email_mode ENUM('off','digest','immediate')` (schema.sql:1285) | +| Immediate vs. digest scheduling | `teamNotify.emailImmediate` and `teamDigestWorker.tick` | +| Digest windowing + clamping | `teamDigestWorker.clampSince`, `MAX_LOOKBACK_MS`, `MAX_ITEMS` | +| One-click unsubscribe | `utils/unsubscribeToken.js` (stateless HMAC) + RFC 8058 `List-Unsubscribe-Post` | +| "Off unless configured" gate | `mailer.isConfigured()` checked *before* the recipient query | +| Multi-sink fan-out from one computed audience | `teamNotify.forumPost` → push + email + Discord bridge | + +This is the prototype of the system being scoped. Building beside it would give the deployment two +unsubscribe mechanisms and two digest workers. Hence decision 2. + +### 0.3 The `uo.house.idoc_warning` payload does not exist on the wire, and one field of it is not knowable + +`house.decay` carries (`docs/link/INTEGRATION.md:207`, +`servuo-plugins/overlay/Scripts/Custom/Bridge/BridgeSweeps.cs:199`): + +``` +serial, from, to, map, x, y, z, region, name, ownerSerial, ownerAcct, ban{x,y,z}, builtOn, lastRefreshed +``` + +Against the brief's example payload: + +| Brief field | Reality | +| --- | --- | +| `house` | ✅ `name` (from the house sign) | +| `location` | ✅ `map`, `x/y/z`, `region`, plus `ban` (where you stand to read the sign) | +| `decay_status` | ✅ `to` | +| `character` | ❌ **not emitted** — but trivially available: `WriteDecay` already reads `house.Owner`, so `owner.Name` is one line | +| `shard` | ❌ **not an event field, and should not become one** — one deployment is one shard; core already has `ctx.settings.getInstanceName()` | +| `next_stage` | ❌ **not emitted** — obtainable, see below | +| `estimated_collapse` | ❌ **not emitted, and not exactly knowable in advance** — see below | + +**Why `estimated_collapse` is the hard one.** ServUO runs the *dynamic* decay system on any modern +shard (`Scripts/Multis/DynamicDecay.cs`: `Enabled => Core.ML`). Under it, each stage's duration is +drawn at random **when that stage is entered** (`GetRandomDuration`, e.g. `Greatly` = 1–2 days, +`IDOC` = 12–24 h). `BaseHouse.NextDecayStage` (`BaseHouse.cs:37`) is therefore an exact, already-persisted +timestamp for the *next* transition — but a collapse time two stages out does not exist yet, even +inside the game. + +Consequences for the design: + +- **`next_stage` is exact and cheap** — emit `NextDecayStage`. +- **`estimated_collapse` is exact only once the house is at IDOC**, where `NextDecayStage` *is* the + collapse time. Before that it can only be an envelope (min/max from the remaining stage table). + Emit it as `estimatedCollapse` only when `to == "IDOC"`, plus an optional + `estimatedCollapseMin`/`Max` envelope earlier — never a single number that reads as a promise. +- Under the legacy static path (`GetOldDecayLevel`, `BaseHouse.cs:205`) decay is a pure function of + `lastRefreshed + DecayPeriod`, and `lastRefreshed` is already on the wire. The **`decayPeriod`** is + not, and should be added so a consumer can compute stages without hardcoding 5 days. + +**And the current mapping fires at the wrong stage for the brief's own example.** The brief's story is +"house becomes greatly damaged". `module-uo/server/config/shardStreams.js` maps `house.decay` to a +stream only when `String(event.to).toUpperCase() === 'IDOC'` — the *final* stage, which is 12–24 h from +collapse. A "greatly damaged" warning needs a `Greatly` transition mapping. That part needs **no +protocol change at all** and can ship long before the bump lands. + +### 0.4 Event-name collision handling already exists — the brief's §6 forward-compat note is already satisfied + +The brief asks for a one-line note that a module-id prefix would be needed *if* multi-module ever +happens. It already happened: + +- `modules/registries.js:namespaced()` **requires** every stream id and announce-leg id to start with + `.`, with a small grandfathering allowlist (`LEGACY_STREAM_IDS`, `LEGACY_LEGS`) for the seven + pre-module-system ids. +- `registries.apply()` throws on any collision, **naming the current holder**, before it commits a + single claim. +- `modules/loader.js` cross-checks mounts, tables and slots across *all* loaded modules + (`for (const other of modules.values())`), so N modules is a supported configuration, not a future one. + +`uo.house.idoc_warning` is therefore already the house style, and the trigger registry gets collision +handling for free by reusing `namespaced()` verbatim. **This is a resolved item, not a forward-compat +note.** The genuine forward-compat note is elsewhere — see §7.3. + +### 0.5 `MODULE_API_VERSION` 1.6.0 is on `main` now, so this needs a real bump + +`MODULE_API.md` §1.1 currently argues that additions may join 1.6.0 in place because "1.6.0 has only +ever been on `edge`". That is stale: `git show main:server/src/modules/version.js` reads `1.6.0`. The +Teams cutover landed it. + +So the engagement additions take **1.7.0** — additions only (`api.registerEventTriggers`, `ctx.events.emit`, +`ctx.inbox.push`), no removal, no changed signature, so minor by the §1.1 table. `module-uo`'s +`coreApi: "^1.3.0"` still resolves. + +--- + +## Part 1 — Current-state map + +### 1.1 Notification system, end to end + +``` + registerNotificationStreams() [modules/registries.js] + │ + config/coreStreams.js (news.post, team.*) ──┤ ← core, via registerCore() + module-uo/config/shardStreams.js (7 ids) ──┘ ← module, via api.* + + catalog read back through registries.allStreams() + │ + GET /auth/me/notifications/streams ──────────────────► web + Android + GET·PUT /auth/me/notifications/subscriptions ─────────► notification_subscriptions + │ + shard event ──► module-uo/utils/shardPush.js ─┤ + news publish ─► posts controller ─────────────┤──► pushDispatch.publish(streamId,{ref,ownerUserId}) + team event ───► utils/teamNotify.js ──────────┘──► pushDispatch.publishToUsers(streamId,{ref,userIds}) + │ + push_devices rows (SSRF-gated endpoints) + │ + POST {stream, ref} ──► ntfy / UnifiedPush ──► app wakes, PULLS content +``` + +**Files.** + +| Concern | File | +| --- | --- | +| Catalog registry (core + modules) | `server/src/modules/registries.js` — `registerNotificationStreams`, `allStreams`, `isValidStream`, `personalStreams` | +| Core's own streams | `server/src/config/coreStreams.js` — `news.post` + four `team.*` | +| Module streams | `module-uo/server/config/shardStreams.js` — seven grandfathered ids + `mapShardEvent` | +| Fan-out | `server/src/utils/pushDispatch.js` — `publish` (all-subscribers / one-owner), `publishToUsers` (computed set), `isAllowedEndpoint` (SSRF gate) | +| Shard → push adapter | `module-uo/server/utils/shardPush.js` — resolves `ownerAcct` → website user via `shardLinks` | +| Team fan-out | `server/src/utils/teamNotify.js` — one audience, three sinks | +| Self-service API | `server/src/router/v1/auth/notifications.controller.js` + `notifications.routes.js` | +| Subscriptions model | `server/src/model/notificationSubs/` | +| Devices model | `server/src/model/pushDevices/` | +| Android surface | `android-app/.../data/api/NotificationsApi.kt`, `dto/NotificationsDto.kt`, `ui/notifications/` | + +**Data model.** + +```sql +-- schema.sql:411 +push_devices(id, user_id, transport ENUM('unifiedpush','fcm'), endpoint, platform, created_at, last_seen_at) +-- schema.sql:428 +notification_subscriptions(user_id, stream_id VARCHAR(64), created_at, PRIMARY KEY(user_id, stream_id)) +-- schema.sql:1285 +team_notification_prefs(user_id, team_id, muted, email_mode ENUM('off','digest','immediate'), last_digest_at, updated_at) +``` + +**Can it cleanly take Email as a third channel? Partly — and the coupling is in one place.** + +- ✅ **The catalog is channel-neutral.** A stream entry is `{ id, label, description, personal, requiresLinkedAccount }`. + Nothing in it is push-specific. It can describe an email or in-app subscription unchanged. +- ✅ **The registry mechanism generalizes.** `stage()` / `apply()` / `namespaced()` are about *claims and + collisions*, not about push. +- ❌ **`notification_subscriptions` has no channel dimension.** `PRIMARY KEY (user_id, stream_id)` means a + subscription is a boolean, and "subscribed" currently means exactly "push me a tickle". +- ❌ **The wire shape is frozen by a shipped client.** `NotificationSubscriptionsDto` is + `{ streams: List }` and the app PUTs the whole set. Turning that array into objects breaks + every installed app. (The DTO ignores *unknown keys*, so purely **additive** fields are safe — this + is recorded in the app's own comments.) A per-channel model must therefore arrive as a **new + endpoint**, with the old one preserved as the push projection. +- ❌ **Email opt-in semantics differ from push, deliberately.** `team_notification_prefs` documents the + asymmetry in its own DDL comment: push is opt-*out* (`muted` defaults 0), email is opt-*IN* + (`email_mode` defaults `'off'`), because digest-by-default would start mailing everyone the moment an + operator connects a mailbox. Any unified model must keep per-channel defaults, not one shared default. + +**Self-hosted ntfy / UnifiedPush integration points** (email must sit beside these, not duplicate them): + +- `pushDispatch.isAllowedEndpoint` — HTTPS-only, private-host denylist, plus an origin allow-set from + `NTFY_ALLOWED_ORIGINS` / `NTFY_BASE_URL`. **There is no hardcoded default host.** Empty allow-set is + the dev fallback. +- `NTFY_PUBLISH_TOKEN` — optional bearer for the relay. +- The **content-free tickle** invariant: `{ stream, ref }` and nothing else, because ntfy is treated as + an untrusted relay. Email deliberately breaks that rule (a mailbox is a destination the recipient + chose) and `teamNotify.js`'s header comment is the standing argument for why the asymmetry is the + security model rather than an inconsistency. **The engagement system must preserve this per channel, + not flatten it.** + +### 1.2 Current email implementation + +**Transport.** `server/src/utils/mailer.js` (267 lines). nodemailer over `smtp.gmail.com:465` with +`auth.type: 'OAuth2'`. Client id/secret are *reused from the `google` auth_providers row*; only the +refresh token is email-specific. `buildTransport()` returns `null` when unconfigured, and every sender +handles that itself. + +**Config.** `email_config` singleton (schema.sql:336) — `provider` (already a `VARCHAR(20)` defaulting +`'gmail_oauth2'`, so the column is ready for a second provider), `enabled`, `sender_email`, `sender_name`, +`refresh_token_enc` (AES-256-GCM via `utils/secretBox.js`), `status`, `status_detail`, `last_verified_at`. +Admin-managed, never env — `docs/website/BACKEND_DESIGN.md` §7. + +**Admin API.** `router/v1/admin/email.router.js` — six routes: `GET·PUT /config`, `GET /connect/start`, +`GET /connect/callback`, `POST /test`, `POST /disconnect`. Client: `client/src/routes/admin/views/EmailDelivery.jsx`. + +**Every caller — the full migration surface.** Six call sites, five sender functions: + +| Caller | Function | Failure contract | +| --- | --- | --- | +| `router/v1/public/public.controller.js:156` (contact form) | `sendContactMessage` | **Never throws when unconfigured** — returns `{sent:false, fallback:'mailto', email}` and the client renders a `mailto:` link | +| `router/v1/admin/emailConfig.controller.js:188` (admin "Send test") | `sendTest` | Throws `NOT_CONFIGURED` / `NO_RECIPIENT`; 502 to the admin | +| `router/v1/admin/invites.controller.js:46` | `sendInvite` | Returns `{sent:false, reason:'NOT_CONFIGURED'}` so the admin gets the accept link to share by hand | +| `router/v1/auth/passwordReset.controller.js:49` | `sendPasswordReset` | Returns `{sent:false, …}`; caller still answers a generic 200 to avoid account enumeration | +| `utils/teamNotify.js:234` (immediate) | `sendTeamNotification` | **Never throws at all** — logged and swallowed; the forum write already returned | +| `utils/teamDigestWorker.js:73` (digest) | `sendTeamNotification` | Same; return value gates the `last_digest_at` stamp | + +Nothing in `website/bot`, `module-uo`, or any other repo sends mail. `mailer` is not on `ctx` — **modules +already cannot send email**, which matches the target architecture. + +**Hardcoded assumptions the abstraction has to remove:** + +1. **One provider, compiled in.** The Gmail host, port and OAuth2 auth type are literals in + `buildTransport()`. `email_config.provider` exists but nothing reads it. +2. **The credential shape is Gmail's.** One `refresh_token_enc` column plus a borrowed OAuth client. + SMTP needs host/port/secure/user/password; SES needs a region and IAM keys; Mailgun/SendGrid need a + domain and an API key. None of those fit the current column set. +3. **One transport built per send.** `buildTransport()` runs on every call — a fresh DB read, a fresh + decrypt and a fresh nodemailer transport per message, with no pooling. Fine for a password reset; + the serial per-recipient loop in `emailImmediate` is explicitly a rate-limit workaround for it. +4. **Text-only, composed inline.** Every body is a template literal inside `mailer.js`. There is **no + HTML part anywhere** and no template storage. `sendTeamNotification` builds its body by pushing lines + into an array. +5. **Synchronous send, no queue, no retry.** A send either succeeds inside the request/tick or is lost. + `recordStatus` writes the last outcome to a singleton column — there is no per-message record, so + "did user X get the IDOC mail?" is unanswerable today. +6. **`recordStatus` is global.** One transient failure sets `email_config.status='error'` for the whole + deployment, from any of six unrelated call sites. +7. **No suppression, no bounce handling, no verification gate.** `users.email` is **not unique** + (SSO addresses repeat) and `users.email_verified` is set to 1 only on invite-accept + (`invite.controller.js:55`) and SSO (`sso.controller.js:208`). Self-registration accepts an address + and leaves it unverified (`auth.controller.js:124`). Today only *transactional* mail goes out, so + this is tolerable; the moment game events drive volume it is a deliverability and complaint problem. + +### 1.2a Removing Gmail OAuth2 — the deletion inventory *(decision 4)* + +Gmail OAuth2 is not a transport we keep beside SMTP. It goes. That is a **subtraction with a live +deployment behind it**, so the exact surface is worth writing down before anyone starts. + +**Server — deleted:** + +| Thing | Where | +| --- | --- | +| `GET /admin/email/connect/start` | `router/v1/admin/email.router.js` | +| `GET /admin/email/connect/callback` | same | +| `connectStart` / `connectCallback` | `router/v1/admin/emailConfig.controller.js` (~half the file's 211 lines) | +| The `email_oauth_tx` signed cookie, the PKCE verifier and CSRF nonce plumbing | same controller | +| `EMAIL_SCOPE = 'https://mail.google.com/ openid email'` | same | +| `googleClient()` — the borrowed `google` auth_providers credential read | same | +| The OAuth2 nodemailer transport (`auth.type: 'OAuth2'`, `smtp.gmail.com:465` literals) | `utils/mailer.js:buildTransport` | +| `emailConfig.getWithSecret()`'s `refreshToken` decrypt | `model/emailConfig/emailConfig.model.js` | + +**Client — deleted:** the "Connect Gmail" button and `connect()` handler, the +`?email_connected` / `?email_error` redirect-banner handling, and the five Gmail-specific error strings +(`bad_state`, `no_client`, `no_refresh_token`, …) in `client/src/routes/admin/views/EmailDelivery.jsx`. +Replaced by an ordinary credential form driven by the transport's `credentialFields` (§3.1). + +**Database — deprecated, not dropped.** `email_config.refresh_token_enc` and `provider` stay as columns +(additive-only discipline; core's `schema.sql` contains exactly one `DROP` and it is documented as such). +They stop being read. A later cleanup PR may drop them once every deployment has booted past the cutover. + +**Three consequences worth naming:** + +1. **SSO is unaffected.** The `google` auth_providers row exists for SSO in its own right; email merely + *borrowed* its client id/secret. Removing the borrow removes a coupling — one of the better side + effects of this decision, since today an admin who rotates the Google SSO secret silently breaks + outbound mail with no indication that the two are related. +2. **`sender_email` changes meaning.** Today it is read back from Google's `userinfo` and is therefore + guaranteed to be an address the mailbox owns. Under SMTP it is **operator-typed**, so nothing stops a + mismatch between the envelope sender and what the SMTP account is permitted to send as — which is a + silent deliverability failure (SPF/DMARC), not an error. The admin "Send test" path has to become the + real verification, and its failure text has to be specific enough to diagnose a rejected `From`. +3. **The live deployment goes dark at cutover unless the operator acts.** UOMysticmoon is connected via + Gmail OAuth2 today. On upgrade, `transport` backfills to `smtp` with **no credentials**, so + `isConfigured()` returns false and every sink politely does nothing — the contact form falls back to + `mailto`, invites surface a copyable link, password resets still answer a generic 200. Nothing breaks + loudly, which is precisely the risk: **email silently stops and nobody is told.** Phase 1 therefore + owes three things: an admin dashboard warning when `transport='smtp'` and credentials are absent, a + release note naming the required action, and `INSTALL.md`-style operator guidance. Gmail itself + remains usable as plain SMTP (`smtp.gmail.com:587` with an app password), which is the shortest + migration path for the existing deployment and should be the documented one. + +### 1.3 Module contract fit + +**The registration surface, as it stands** (`modules/registries.js`, `MODULE_API.md` §2.4): + +| Call | Shape | Cardinality | +| --- | --- | --- | +| `registerRoutes` | tier → prefix → router | declared in `module.json`, cross-checked | +| `registerExtension(slot, router)` | fills a **core-declared** slot | one filler per slot | +| `registerNotificationStreams([…])` | push catalog entries | many, `.`-prefixed | +| `registerAnnounceLeg({leg, label, dispatch, classify})` | a **delivery leg** with retry classification | many, `.`-prefixed | +| `registerPostHook({onSaved, onDeleted})` | idempotent state mirroring | one per owner | +| `registerTeamProvider({…})` | core **calls the module and waits** | one per deployment | +| `registerSlashCommands([…])` | definition travels, handler stays | many, *not* namespaced (Discord grammar) | + +**Is there a natural extension point? Yes — and `registerAnnounceLeg` is the closest structural match, +but for the *delivery* half, not the *trigger* half.** + +The engagement system needs **two** things a module does not have today: + +1. **A way to declare a domain event and its data contract** — nothing like this exists. `registerNotificationStreams` + declares a *subscription toggle*; it carries a label and two booleans, and no statement whatsoever + about payload. A module cannot tell core what a `uo.house.idoc_warning` *contains*. +2. **A way to emit one.** Today `ctx.push.publish(streamId, {ref, ownerUserId})` is the only outbound + path, and it is deliberately content-free. A module that wanted to send a *rendered* message has to + go through `pushDispatch`, which will not carry the data. + +So this needs a **new registration surface**, not a reuse. It should be modelled on `registerNotificationStreams` +(shape-checked at the call, collision-checked at `apply()`, `.`-prefixed) rather than on +`registerAnnounceLeg` (which is a *core-calls-module* dispatch with retry classification — the wrong +direction: a module *reports* an event, it does not deliver one). + +**What module-uo exposes to core today.** Only what the registries take: seven stream ids, one announce +leg, one extension router, a Team provider, one slash command, five route mounts. Everything else — 27 +`shard_*` tables, the sidecar client, the visibility framework — is module-internal (`MODULE_API.md` §1.2). +There is **no data channel from a module into core carrying structured game data**. `ctx.teams.activity.push` +is the nearest thing, and it is instructive: core stores `summary` **already rendered by the module**, +because core cannot phrase a sentence in a vocabulary it does not know, and `kind`/`payload` are opaque. + +The engagement system deliberately takes the **opposite** position — core *does* interpolate module data +into a template — which is only safe because the operator authors the template and the module *declares* +the variables. That is the whole reason §4.3's variable contract has to exist rather than being optional. + +### 1.4 Job, scheduling and queue infrastructure that already exists + +**There is no cron. There is no Redis, no BullMQ.** The stack has exactly two patterns: + +**(a) In-process `setInterval` + `unref()` + `stop()`, wired into `server.js` start/shutdown.** Six of them: +`announceWorker`, `teamDigestWorker`, `teamActivityPrune`, `teamForumUploadSweep`, `teamVoiceSync`, and +`middleware/botScore`'s sweeper. + +**(b) A durable job table with per-leg backoff** — `announce_jobs` + `announce_job_legs` (schema.sql:737/757), +swept by `announceWorker.tick`. This is a real outbox: `status`, `attempts`, `last_error`, +`next_attempt_at`, `INDEX idx_announce_leg_due (status, next_attempt_at)`, and a `classify()` that maps a +delivery result to done / retry / terminal. **It was explicitly designed so a module can add a delivery +leg without altering a core table** — the child-table shape and the `VARCHAR` (not `ENUM`) `leg` column +are both justified in the DDL comment on exactly those grounds. + +**Which of the brief's two scheduling use cases each pattern serves:** + +- **(a) Delayed send per event** — "wait 30 min in case the player fixes it". Needs a durable row with a + `due_at`, and — the part the brief does not name but which is the actual point — the ability to + **cancel** a pending row when a later event resolves the condition. `announce_jobs` is the exact + precedent; this needs its own table because a module cannot alter a core one and the payload differs. +- **(b) Batched / digest** — `teamDigestWorker` already does this, and its header comment is the design + argument: it **computes at send time and keeps no queue**, whose three consequences are (1) a + deployment down for two days sends *one* digest, not a replay, (2) content hidden after it was written + is not in the query so not in the mail, and (3) **a user who lost access between the post and the send + is no longer in the recipient set** — which it calls out as the one that would have been a security bug. + +**That split is the answer to the brief's §4 scheduling question: (a) is a queue, (b) must not be.** +Copying (a) for digests would reintroduce all three problems. + +**One gap in both patterns: neither is multi-instance safe.** No advisory lock, no leader election, no +`SELECT … FOR UPDATE SKIP LOCKED`. Two app containers means two digest sweeps and two announce workers. +The current deployment is single-instance (`website/docker-compose.yml`), so this is latent — but an +engagement mailer doubles messages rather than doubling reads, so it becomes visible here first. + +**Reusable primitives worth naming:** + +- `utils/unsubscribeToken.js` — a stateless HMAC whose whole capability is "set `muted` for one (user, Team) + pair". Generalizes to (user, channel, trigger) with no structural change. +- `utils/settingsJson.js` — the fail-safe JSON-settings parse (malformed ⇒ *absent*, never an error). +- `blocks/registry.js` + `blocks/types/*` + `sanitizeBlocks.js` + `validateBlocks.js` — a versioned, + schema-validated, sanitize-on-save visual block system already driving `pages.blocks` (MEDIUMTEXT JSON). + This is the template editor's foundation; see §4.4. +- `utils/secretBox.js` — AES-256-GCM for provider credentials at rest. +- `model/settings/settings.model.js` + the `settings` key/value table — right for a handful of scalars, + **wrong for cooldowns** (see §4.1). + +--- + +## Part 2 — Gap list against the target architecture + +``` +Game Module ──► Domain Events + Data ──► Core ──► Engagement ──► Preferences ──► Template ──► Delivery +``` + +| # | Layer | Gap | Severity | +| --- | --- | --- | --- | +| G1 | Module → events | No way for a module to **declare** a domain event or its payload contract | Blocking | +| G2 | Module → events | No way for a module to **emit** one carrying data (`ctx.push.publish` is content-free by design) | Blocking | +| G3 | Events → Core | No event **catalog** surface for the admin UI to enumerate triggers | Blocking | +| G4 | Engagement | No **rules** concept at all — today a trigger's consequence is hardcoded in the emitting file | Blocking | +| G5 | Engagement | No **cooldown / rate-limit** state of any kind, per-recipient or otherwise | Blocking (brief calls this out as pre-first-trigger) | +| G6 | Engagement | No **delayed-send** queue and no cancellation | High | +| G7 | Engagement | Digest exists but is **Team-shaped**, not generic (`last_digest_at` lives on `team_notification_prefs`) | High | +| G8 | Preferences | `notification_subscriptions` has **no channel dimension**; the shipped app's wire shape is frozen | Blocking | +| G9 | Preferences | Per-channel **defaults differ** (push opt-out, email opt-in) and there is nowhere to express that generically | High | +| G10 | Preferences | Unsubscribe is **Team-scoped** (`unsubscribeToken.sign(userId, teamId)`) | Medium | +| G11 | Template | **No template storage, no HTML part, no renderer, no preview, no plain-text fallback.** Every body is a string literal in `mailer.js` | Blocking | +| G12 | Template | No **variable contract** — nothing declares what a template may interpolate | Blocking (see §4.3) | +| G13 | Delivery | **One hardcoded provider**; `email_config.provider` is written but never read | Blocking | +| G14 | Delivery | Credential schema is Gmail-shaped (one refresh token + a borrowed OAuth client) | Blocking | +| G15 | Delivery | No per-message record — **no send log, no delivery status, no audit** | High | +| G16 | Delivery | No **suppression list**, no bounce/complaint handling, no unverified-address policy | High | +| G17 | Channels | **In-app channel does not exist** — no table, no read API, no web surface, no app surface | Blocking (in scope) | +| G18 | Infra | Workers are **not multi-instance safe** — latent today, doubles *messages* under engagement | Medium | +| G19 | Contract | `MODULE_API_VERSION` must go 1.6.0 → **1.7.0** (§0.5) | Process | +| G20 | Wire | `house.decay` lacks `ownerName`, `nextStage`, `decayPeriod`, collapse estimate (§0.3) | High (in scope) | +| G21 | Ops | No **preview/test-send** path for a template against a real trigger payload | Medium | +| G22 | Delivery | Removing Gmail OAuth2 leaves the live deployment **silently unconfigured** — every sink degrades quietly, so email stops with no signal (§1.2a) | High (in scope) | +| G23 | Template | No **seeded default templates** — without them, "add a trigger" implies "and now author a template", and a fresh install mails nothing (§4.6.1) | High (in scope) | +| G24 | Engagement | An audience **ceiling** per trigger. `shardStreams.js` already filters sensitive kinds off the public push path; the engine needs the equivalent or a rule can widen a staff-only trigger to everyone (§8.6) | Blocking (security) | +| G25 | Engagement | No **time-based** trigger kind — "nothing happened for 30 days" is a periodic evaluator, not an event (§8.5) | Medium (design in Phase 2, build later) | + +--- + +## Part 3 — The delivery abstraction (brief §5) + +### 3.1 Name it `DeliveryChannel`, and split *channel* from *transport* + +The brief asks whether the interface should be `EmailProvider` or something more generic. **Neither +alone.** Two axes are being conflated, and the current code conflates them too: + +- A **channel** is *what kind of sink this is* — email, push, in-app, later Discord DM. It determines the + address kind (mailbox / endpoint URL / user id / snowflake), the render contract (subject + HTML + text + vs. `{stream, ref}` vs. an embed), the preference semantics, and whether content may ride at all. +- A **transport** is *how one channel actually delivers* — SMTP / Gmail-OAuth2 / Mailgun / SES / SendGrid + for email; ntfy-UnifiedPush / FCM for push. + +Push already has this shape and nobody named it: `push_devices.transport ENUM('unifiedpush','fcm')` is a +transport column on a channel that has exactly one implementation today. + +**Recommended surface:** + +```js +// core-internal registry, mirroring modules/registries.js's shape +registerDeliveryChannel({ + id: 'email', // 'email' | 'push' | 'inapp' | later 'discord.dm' + label: 'Email', + carriesContent: true, // false for push — enforces the tickle invariant structurally + defaultMode: 'off', // email opt-IN, push opt-OUT — G9, expressed here once + supportsDigest: true, // in-app and push are instant-only in v1 + addressFor(userId), // → [{ address, meta }] ; email reads users.email, push reads push_devices + render(template, vars, ctx), // → the channel's own payload shape + deliver(address, payload), // → { ok, retryable, error } — never throws +}) + +registerMailTransport({ + id: 'smtp', // 'smtp' | 'mailgun' | 'ses' | 'sendgrid' — NOT gmail_oauth2 (removed) + label: 'SMTP', + credentialFields: [...], // drives the admin form AND the encrypted credential blob + build(config), // → a nodemailer transport (or an API client) + verify(config), // → the admin "Send test" path +}) +``` + +**Answering the brief's actual question: no, this will not need a breaking rename when Discord DM +arrives.** A Discord DM is a `registerDeliveryChannel({ id: 'discord.dm', carriesContent: true, … })` +whose `deliver` calls `utils/botInternalClient.js` — the bot-internal API already exists and +`utils/teamBridge.js` is the working precedent for core handing a composed message to the bot. Nothing +in the interface above says "email". + +**One vocabulary warning.** The announce pipeline already calls a delivery a **leg** +(`registerAnnounceLeg`, `announce_job_legs.leg`). `channel` and `leg` will coexist and mean *nearly* the +same thing. They should stay distinct rather than being unified: a leg is a **one-shot delivery of one +artifact** with retry and terminal classification; a channel is a **per-recipient sink** with preferences, +addresses and digest semantics. `teamBridge.js`'s header already argues this distinction for the Discord +case ("one-shot, not queued… a notification is the moment it describes"). The design doc should say so +explicitly so nobody "tidies" them together later. + +### 3.2 No phone-home — the existing posture is already correct, and the registry must preserve it + +Confirmed, and there is nothing to fix — only something to not break: + +- `email_config` is **DB-backed and admin-managed**, never env (`BACKEND_DESIGN.md` §7). There is no + default host, no default sender, and `status` starts `'unconfigured'`. +- `pushDispatch.allowedOrigins()` reads `NTFY_ALLOWED_ORIGINS` / `NTFY_BASE_URL` and **returns empty when + neither is set**. No Runic Gateway host appears anywhere in it. +- `mailer.isConfigured()` gates every sink, and `teamNotify.emailImmediate` checks it *before* the + recipient query so an unconfigured deployment pays nothing. + +**Rules to carry into the abstraction:** + +1. No transport may ship a default host, endpoint, API base or sender. A transport with no operator + configuration is `unconfigured` and its channel is **off**, not defaulting to anything. +2. No engagement code may read an env var naming an external service that the operator did not set. +3. The "off unless configured" gate is checked before recipient resolution, per channel. +4. A CI guardrail: extend `scripts/checkModuleIdentifiers.js`'s sibling pattern with a check that no + file under `server/src/engagement/` contains a bare external hostname literal. (Cheap; the check + pattern and its self-test discipline already exist — see `test/checkModuleIdentifiers.test.js`, which + feeds the checker code it *must* reject precisely so a check cannot silently stop checking.) + +--- + +## Part 4 — Proposed schema additions + +All additive. All `CREATE TABLE IF NOT EXISTS` / `ALTER … ADD COLUMN IF NOT EXISTS`, replayed on every +boot, per `MODULE_API.md` §2.6's rules (which core's own `schema.sql` follows too). **Core tables, no +prefix** — every one of these is game-agnostic. + +> **MariaDB trap, already learned twice in this codebase:** a `PRIMARY KEY` column is coerced `NOT NULL`, +> so "NULL means the default row" is unrepresentable in a PK. `team_integration_config` (schema.sql:1335) +> and `teams.active_key` both work around it with a surrogate key plus a generated column folding NULL onto +> a sentinel. Two tables below need the same treatment; both are flagged. + +### 4.1 Cooldowns — its own table, not `settings` + +The brief asks whether the `settings.model.js` JSON-value pattern is a reasonable fit. **No.** +`settings` is `(key VARCHAR(64) PRIMARY KEY, value TEXT)` — a single-row-per-key store read whole. Cooldown +state is high-cardinality (recipients × rules × subjects), written on every fire, and queried as +"is this one pair still cooling?". A JSON blob under one key would be a read-modify-write of the entire +deployment's cooldown state on every event, with a lost-update race between two concurrent triggers. It is +the wrong shape by an order of magnitude. + +```sql +CREATE TABLE IF NOT EXISTS engagement_cooldowns ( + rule_id INT NOT NULL, + user_id INT NOT NULL, + -- The SUBJECT the cooldown is about, opaque to core: a house serial, a vendor id, ''. + -- NOT NULL with a '' default, because this is a PRIMARY KEY column and MariaDB + -- would coerce a NULL one anyway. '' is "this rule cools per user, not per subject". + subject_key VARCHAR(190) NOT NULL DEFAULT '', + last_fired_at DATETIME NOT NULL, + fire_count INT NOT NULL DEFAULT 1, + PRIMARY KEY (rule_id, user_id, subject_key), + CONSTRAINT fk_engc_rule FOREIGN KEY (rule_id) REFERENCES engagement_rules(id) ON DELETE CASCADE, + CONSTRAINT fk_engc_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + INDEX idx_engc_sweep (last_fired_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +``` + +**Why `subject_key` is not optional.** "One IDOC mail per player per day" is the wrong rule — a player +with four houses decaying should hear about all four, once each. Cooling per (rule, user) alone silently +drops three of them. The module supplies `subject` on emit; core stores it opaquely. + +`INDEX idx_engc_sweep (last_fired_at)` exists so a prune worker can drop rows older than the longest +configured cooldown — otherwise this table grows without bound, which is the failure mode +`teamActivityPrune` was written for. + +**The check is `INSERT … ON DUPLICATE KEY UPDATE` guarded on the interval**, in one statement, so two +concurrent emits cannot both pass a read-then-write check. + +### 4.2 Scheduling — a queue for delay, and deliberately no queue for digest + +**(a) Delayed send ⇒ `engagement_outbox`.** Modelled on `announce_jobs`/`announce_job_legs`. + +```sql +CREATE TABLE IF NOT EXISTS engagement_outbox ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + rule_id INT NOT NULL, + trigger_id VARCHAR(96) NOT NULL, -- denormalized; survives a rule edit + user_id INT NOT NULL, + channel VARCHAR(32) NOT NULL, -- 'email' | 'push' | 'inapp' | … VARCHAR, never ENUM + subject_key VARCHAR(190) NOT NULL DEFAULT '', + payload JSON NOT NULL, -- the module's declared variables, snapshotted at emit + -- Idempotent enqueue. A sidecar reconnect that replays the same event must not + -- produce a second mail. Same reasoning as ctx.teams.activity.push's dedupeKey. + dedupe_key VARCHAR(190) NULL, + status ENUM('scheduled','sending','sent','failed','cancelled','suppressed') NOT NULL DEFAULT 'scheduled', + due_at DATETIME NOT NULL, + attempts SMALLINT NOT NULL DEFAULT 0, + last_error TEXT NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + sent_at DATETIME NULL, + CONSTRAINT fk_engo_rule FOREIGN KEY (rule_id) REFERENCES engagement_rules(id) ON DELETE CASCADE, + CONSTRAINT fk_engo_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + UNIQUE KEY uq_engo_dedupe (dedupe_key), + INDEX idx_engo_due (status, due_at), + INDEX idx_engo_cancel (rule_id, user_id, subject_key, status) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +``` + +Three things this buys that a straight send does not: + +- **`due_at` is the 30-minute grace window.** The worker sweeps `status='scheduled' AND due_at <= now`. +- **`status='cancelled'` is the actual point of that window.** `idx_engo_cancel` is what a *resolving* + event queries: a `house.decay` back up to `LikeNew` cancels every scheduled row for that + (rule, user, house). Without cancellation, a delay is just a late mail. +- **`dedupe_key UNIQUE` makes replay safe.** The sidecar has no schema-migration mechanism and a + reconnect backfills; an at-least-once feed must not become an at-least-once mailer. + +`channel` is `VARCHAR(32)` and not an `ENUM` for exactly the reason `announce_job_legs.leg` is — +the channel set is data, and a module (or a later core channel) must not require an `ALTER`. + +**(b) Digest ⇒ no queue.** Keep `teamDigestWorker`'s compute-at-send-time design and generalize its +state, not its absence of one: + +```sql +CREATE TABLE IF NOT EXISTS engagement_digest_state ( + user_id INT NOT NULL, + channel VARCHAR(32) NOT NULL, + scope_key VARCHAR(190) NOT NULL DEFAULT '', -- '' = deployment-wide; a Team id for the Teams case + last_digest_at DATETIME NULL, + PRIMARY KEY (user_id, channel, scope_key), + CONSTRAINT fk_engd_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + INDEX idx_engd_due (channel, last_digest_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +``` + +`team_notification_prefs.last_digest_at` backfills into this with `channel='email'`, +`scope_key = team_id`. The three properties from `teamDigestWorker`'s header comment must be preserved +verbatim by the generic worker, and the third one (a user who lost access is no longer in the recipient +set) should get its own named test, the way the Teams phase-5 work gave the leader-can't-see-reports rule +its own test. + +### 4.3 The template variable contract — a code-declared schema, mirrored to a checked-in manifest + +The brief asks whether anything in the codebase already does this. **Two things do, and they are the two +halves of the right answer:** + +- **`blocks/registry.js`** — a registered definition carries `version` (a *prop-schema* version, bumped + when props change so a migration can transform older blocks), `schema: (props) => [errors]`, and + `sanitize: (props) => props` run on save *after* validation. That is the validation half. +- **`server/scripts/routeManifest.js` + `routes.manifest.json`, checked in CI with `--check`** — a + generated artifact committed to the repo, whose diff is the review signal. That is the drift half. + (`module-uo` carries its own `routes.manifest.json` for the same reason, and ships a prebuilt + `swagger-fragment.json` because core never has its sources to analyse — `MODULE_API.md` §6.1a.) + +**Proposal: the trigger declaration carries its variables, and a generated manifest freezes them.** + +```js +api.registerEventTriggers([{ + id: 'uo.house.idoc_warning', // .-prefixed, checked by the existing namespaced() + label: 'House approaching collapse', + description: 'A player house dropped into a late decay stage.', + subjectKey: 'house', // which variable identifies the subject, for cooldowns + audience: 'owner', // 'owner' | 'subscribers' | 'computed' + variables: [ + { name: 'character', type: 'string', required: true, example: 'Darrow' }, + { name: 'house', type: 'string', required: true, example: 'The Silver Anvil' }, + { name: 'location', type: 'string', required: true, example: 'Britain, Trammel (1119, 1794)' }, + { name: 'decayStatus', type: 'string', required: true, example: 'Greatly' }, + { name: 'nextStage', type: 'datetime', required: false, example: '2026-08-30T04:00:00Z' }, + { name: 'estimatedCollapse',type: 'datetime', required: false, example: '2026-09-01T04:00:00Z' }, + ], +}]) +``` + +Four properties, each with a reason: + +1. **Validated at emit, not at render.** `ctx.events.emit` checks the payload against the declaration. + A missing `required` variable or a wrong type is **dropped and logged in production, thrown in + development** — the same posture `ctx.teams.activity.push` takes ("a malformed item is dropped and + logged"), because this is called from inside a game-event handler and a storage problem of core's must + not become the module's control flow. +2. **The editor reads it, so autocomplete is real.** `GET /admin/engagement/triggers` serves the + declarations; the template editor offers exactly those names and refuses to save a template + referencing one that is not declared. That is G12 closed — the editor never blindly interpolates + module JSON. +3. **`example` is not decoration — it is the preview and the test-send.** Without it, previewing a + template requires a live game event, which is the reason template systems go untested. +4. **Drift is caught by a committed manifest.** `npm run engagement:manifest` writes + `server/engagement-triggers.json` (core's) and CI runs it with `--check`, exactly as + `routes:manifest -- --check` already gates the URL surface. Changing a variable's name or type + without regenerating is a red build; the diff is what a reviewer reads. **A module ships its own + prebuilt `engagement-triggers.json` in its bundle**, for the same reason it ships a prebuilt + swagger fragment: core never has its sources. + +**Versioning.** A variable's *addition* is additive and needs nothing. A **rename or a type change** breaks +every stored template referencing it, so a trigger declaration carries `version`, bumped like a block's +prop-schema version, and templates store the trigger version they were authored against. A template +pinned to an older version renders with a warning in the admin list rather than silently interpolating +`undefined`. + +### 4.4 Templates — reuse the block registry, do not build a second editor + +```sql +CREATE TABLE IF NOT EXISTS engagement_templates ( + id INT AUTO_INCREMENT PRIMARY KEY, + `key` VARCHAR(96) NOT NULL UNIQUE, -- stable id a rule points at + name VARCHAR(160) NOT NULL, + trigger_id VARCHAR(96) NULL, -- NULL = a reusable/shared template + trigger_version INT NULL, -- what its variables were authored against (§4.3) + channel VARCHAR(32) NOT NULL, -- one template per channel; a rule names a set + subject VARCHAR(300) NULL, -- email only; may interpolate + blocks MEDIUMTEXT NOT NULL, -- JSON array — the pages.blocks pattern + text_body MEDIUMTEXT NULL, -- authored plain-text override; else generated + status ENUM('draft','published') NOT NULL DEFAULT 'draft', + -- A seeded template that the system itself depends on (password reset, invite). + -- Editable, NOT deletable — the pages.protected flag, for the same reason. + protected TINYINT(1) NOT NULL DEFAULT 0, + -- Which seed revision this row came from, and whether an operator has since + -- touched it. Together they let a later release ship an improved default + -- WITHOUT overwriting an operator's edits. See §4.6. + seed_key VARCHAR(96) NULL, + seed_version INT NULL, + customized TINYINT(1) NOT NULL DEFAULT 0, + updated_by INT NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + CONSTRAINT fk_engt_user FOREIGN KEY (updated_by) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_engt_trigger (trigger_id, channel, status) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +``` + +**Why blocks and not raw HTML.** The brief asks for a visual editor taking inspiration from the CMS +page builder and the hero editor. The CMS builder is *already* a registry-driven block system with +server-side prop validation and sanitize-on-save (`blocks/registry.js`, `blocks/types/*`, +`sanitizeBlocks.js`). Storing raw operator HTML would give up all of that and hand the renderer an +injection surface. + +**But email needs its own block set, not the page one.** Page blocks emit modern CSS that mail clients +do not support. Register a parallel family — `email.heading`, `email.text`, `email.button`, +`email.divider`, `email.image`, `email.itemList` — that render to table-based, inline-styled HTML. The +registry is designed for exactly this: "adding a block later means adding ONE entry". + +**Plain-text fallback is generated by default, overridable per template.** Every block type gets a +`toText(props)` alongside its renderer, so a text part always exists. `teamNotify.excerpt()` is the +existing markup-to-text helper and should move into that family rather than being duplicated. + +**A `text_body` that is empty for a published template is a save-time error, not a runtime one** — a +mail with no text part is a spam-filter signal, and finding out at send time means finding out from a +deliverability report. + +### 4.5 Rules, channel preferences, send log, suppression + +```sql +-- What an operator actually configures: trigger → audience → template → timing. +CREATE TABLE IF NOT EXISTS engagement_rules ( + id INT AUTO_INCREMENT PRIMARY KEY, + trigger_id VARCHAR(96) NOT NULL, + name VARCHAR(160) NOT NULL, + enabled TINYINT(1) NOT NULL DEFAULT 0, -- OFF by default; an operator turns it on + audience VARCHAR(32) NOT NULL DEFAULT 'owner', + channels JSON NOT NULL, -- ['email','inapp'] — a rule may span channels + template_keys JSON NOT NULL, -- { email: 'idoc-warning', inapp: 'idoc-warning-short' } + conditions JSON NULL, -- declared-variable predicates, e.g. decayStatus in [Greatly, IDOC] + cooldown_seconds INT NOT NULL DEFAULT 0, + delay_seconds INT NOT NULL DEFAULT 0, -- the grace window (§4.2a) + cancel_on JSON NULL, -- trigger ids that cancel a pending row for the same subject + updated_by INT NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + CONSTRAINT fk_engr_user FOREIGN KEY (updated_by) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_engr_trigger (trigger_id, enabled) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- G8/G9: the channel dimension notification_subscriptions lacks. +CREATE TABLE IF NOT EXISTS notification_channel_prefs ( + user_id INT NOT NULL, + stream_id VARCHAR(64) NOT NULL, -- a stream OR a trigger id; one namespace, see §7.2 + channel VARCHAR(32) NOT NULL, + mode ENUM('off','instant','digest') NOT NULL DEFAULT 'off', + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, stream_id, channel), + CONSTRAINT fk_ncp_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + INDEX idx_ncp_channel (channel, mode) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- G15: per-message record. Today "did user X get the mail?" is unanswerable. +CREATE TABLE IF NOT EXISTS engagement_sends ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + outbox_id BIGINT NULL, + rule_id INT NULL, + trigger_id VARCHAR(96) NOT NULL, + user_id INT NULL, -- SET NULL, so the log survives an account deletion + channel VARCHAR(32) NOT NULL, + transport VARCHAR(32) NULL, -- which mail transport actually carried it + address_hash CHAR(64) NULL, -- sha256; the log must not be a second address book + status ENUM('sent','failed','suppressed','bounced','complained') NOT NULL, + detail VARCHAR(500) NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_engs_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_engs_trigger (trigger_id, created_at), + INDEX idx_engs_user (user_id, created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- G16. Keyed on the ADDRESS, not the user: users.email is not unique. +CREATE TABLE IF NOT EXISTS engagement_suppressions ( + address_hash CHAR(64) NOT NULL PRIMARY KEY, -- sha256 of the lowercased address + channel VARCHAR(32) NOT NULL DEFAULT 'email', + reason ENUM('bounce','complaint','manual','unverified') NOT NULL, + detail VARCHAR(500) NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- G17: the in-app inbox. Core, game-agnostic, content-carrying. +CREATE TABLE IF NOT EXISTS user_notifications ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + user_id INT NOT NULL, + trigger_id VARCHAR(96) NOT NULL, + title VARCHAR(300) NOT NULL, + body TEXT NULL, -- rendered by the inapp template, sanitized on write + url VARCHAR(500) NULL, -- relative only, validated like pageUrlTemplate + dedupe_key VARCHAR(190) NULL, + read_at DATETIME NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_un_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + UNIQUE KEY uq_un_dedupe (user_id, dedupe_key), + INDEX idx_un_unread (user_id, read_at, created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +``` + +**`email_config` grows rather than being replaced** (additive-only discipline): + +```sql +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS transport VARCHAR(32) NOT NULL DEFAULT 'smtp'; +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS credential_enc TEXT NULL; -- AES-GCM JSON blob, transport-shaped +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS reply_to VARCHAR(255) NULL; +``` + +`transport` defaults to `'smtp'` because Gmail OAuth2 is gone (§1.2a) — there is no longer a transport +for an existing `provider='gmail_oauth2'` row to backfill *into*, so the upgrade lands every deployment +on SMTP with empty credentials and an explicit admin warning rather than on a transport that no longer +exists. `provider` and `refresh_token_enc` stay as dead columns (additive-only) and stop being read. + +A single opaque `credential_enc` JSON blob is better than one column per provider field: SMTP, SES and +Mailgun have disjoint credential shapes and the transport's `credentialFields` already describes its own. +It also means adding Mailgun later is a registration plus an admin form, with **no schema change at all**. + +**The backfill of `notification_subscriptions` → `notification_channel_prefs`** follows the precedent +already in `schema.sql:770` (the `announce_jobs` → `announce_job_legs` migration): an `INSERT IGNORE … +SELECT` guarded so replay on every boot is a no-op after the first. + +```sql +INSERT IGNORE INTO notification_channel_prefs (user_id, stream_id, channel, mode) + SELECT user_id, stream_id, 'push', 'instant' FROM notification_subscriptions; +``` + +### 4.6 Basic templates and the editor *(decision 5)* + +Two halves, and the first is the one that decides whether the second gets used. + +#### 4.6.1 The seeded set — a fresh deployment mails correctly before anyone opens the editor + +Today every message body is a template literal inside `mailer.js`. Phase 5 moves them into +`engagement_templates` **as seeded rows**, so the migration is a relocation rather than a regression: +nothing that sends mail today starts depending on an operator authoring something first. + +Seeded on boot by an idempotent seeder alongside `db/seed.js`'s admin seed — `INSERT … ON DUPLICATE KEY +UPDATE` keyed on `seed_key`, **and it refuses to overwrite a row whose `customized` flag is set**. + +**Transactional (`protected = 1`, editable but not deletable — the system breaks without them):** + +| `seed_key` | Replaces | Variables | +| --- | --- | --- | +| `auth.password-reset` | `mailer.sendPasswordReset` | `username`, `resetUrl`, `expiresIn`, `siteName` | +| `auth.invite` | `mailer.sendInvite` | `acceptUrl`, `role`, `invitedByName`, `siteName` | +| `auth.email-verify` | *(new — Phase 9)* | `username`, `verifyUrl`, `expiresIn` | +| `admin.contact-message` | `mailer.sendContactMessage` | `fromName`, `fromEmail`, `message` | +| `admin.test` | `mailer.sendTest` | `siteName`, `transport`, `sentAt` | + +**Notification (`protected = 0`, replaceable):** + +| `seed_key` | Replaces | Variables | +| --- | --- | --- | +| `notify.event` | the generic single-event mail | `title`, `intro`, `items[]`, `actionUrl`, `unsubscribeUrl` | +| `notify.digest` | `teamDigestWorker`'s body | `intro`, `periodLabel`, `items[]`, `moreCount`, `scopeUrl`, `unsubscribeUrl` | +| `notify.team-post` | `mailer.sendTeamNotification` immediate | `teamName`, `authorName`, `threadTitle`, `excerpt`, `threadUrl` | +| `inapp.event` | *(new)* — the in-app channel's short form | `title`, `body`, `url` | + +**Three properties of the seeded set that are design, not packaging:** + +1. **`notify.event` and `notify.digest` are generic on purpose.** A new trigger — from core or from any + module — renders through them with no authoring at all, because their variables are structural + (`title`, `intro`, `items[]`) rather than domain-specific. An operator who wants a bespoke IDOC mail + writes one; an operator who does not still gets a sane one. **This is what stops "add a trigger" + from meaning "and now write a template."** +2. **They are branded from data, not hardcoded.** `BRAND_*` env and the `theme_visual` / `brand_assets` + settings already drive the site's colours and logo (`THEMING_AND_NAV.md` §4.4); the seeded templates + read the same resolved values, so one prebuilt image running as any shard mails in that shard's + colours. No template contains a literal hex code or a logo URL. +3. **A later release can improve a default without stealing an operator's work.** `seed_version` + + `customized` is the whole mechanism: on boot, a seed whose version is newer updates rows where + `customized = 0` and **skips** rows where it is 1, surfacing "an updated default is available" in the + admin list instead. Same posture `settingsJson` takes — a stored value that is unusable is treated as + absent, never as an error. + +#### 4.6.2 The editor + +Built on the existing block machinery (`blocks/registry.js`, the prop panels, `sanitizeBlocks.js`), +with an `email.*` block family (§4.4) — **not a second editor**. What it adds over the page builder: + +- **A variable palette from the trigger declaration.** The right-hand panel lists exactly the variables + §4.3 declares for this template's trigger, with type and example. Inserting one writes a token; it + is never free-text. A template referencing an undeclared variable is **refused at save, naming the + variable** — the editor validates, it does not blindly interpolate module JSON. +- **Live preview from `example` values.** No live game event needed. This is the reason `example` is a + required part of the trigger declaration rather than documentation. +- **A side-by-side HTML / plain-text view.** The text part is generated from each block's `toText`, and + is overridable per template. A published template with an empty text part is a **save-time error**. +- **Test send to an address of the admin's choosing**, through the configured transport, recorded in + `engagement_sends` like any other message. +- **Three preview widths** (desktop / mobile / plain-text) and a dark-mode preview, because mail clients + invert backgrounds and a light-only template renders as unreadable dark-on-dark in about a third of + inboxes. +- **A duplicate action**, which is how an operator customizes a `protected` template safely: duplicate, + edit, point the rule at the copy, leave the original intact. + +**Security posture, stated because this is the one new place operator HTML reaches a rendered surface:** +blocks are validated and sanitized on **write** (the existing `validateBlocks` → `sanitize` order), the +preview renders in a sandboxed iframe with no `allow-scripts`, and variable interpolation is +**HTML-escaped by default** with no raw-HTML variable type in v1. A module supplies data; it does not +supply markup. + +--- + +## Part 5 — The module registration mechanism + +### 5.1 Two additions to the contract, both modelled on what already works + +```js +// api — what the module registers (MODULE_API.md §2.4). Modelled on +// registerNotificationStreams: shape-checked at the call, collision-checked at +// apply(), .-prefixed by the existing namespaced() helper. +api.registerEventTriggers([{ id, label, description, variables, subjectKey, audience, version }]) + +// ctx — what core hands the module (§2.3). Modelled on ctx.teams.activity.push: +// fire-and-forget, never throws, never rejects, malformed input dropped and logged. +ctx.events.emit(triggerId, { subject, data, ownerUserId?, dedupeKey?, occurredAt? }) + +// ctx — the in-app sink, for a module that wants to write the inbox directly +// without a rule. Optional; most modules will only emit. +ctx.inbox.push(userId, { triggerId, title, body, url, dedupeKey }) +``` + +**Why a new surface rather than extending `registerNotificationStreams`.** A stream entry is a +*subscription toggle* — label plus two booleans, with no statement about payload. A trigger is a *data +contract*. Overloading the stream entry with a `variables` array would make every existing push stream +look like it has an (empty) payload contract, and would put the emit path for content-free tickles and +content-carrying events through one function whose behaviour depends on which fields the caller filled +in. The two should stay separate for the same reason `registerPostHook` was kept out of +`registerAnnounceLeg` ("a leg is a one-shot DELIVERY with retry and classification; a post hook maintains +idempotent STATE" — `registries.js`). + +**What core reuses verbatim:** `stage()` / `apply()`'s validate-then-commit-per-registrant discipline, +`namespaced()` for the `.` prefix, the collision message that names the current holder, and the +rule that nothing a registrant claims takes effect until the whole registrant is known good. + +**Core registers its own triggers through the same door**, in `registerCore()`, exactly as it does for +streams and the Discord leg. That is not ceremony — `registries.js`'s header states the reason: "a registry +only core's hardcoded base bypasses is a registry whose first real exercise is a module, which is the drift +this PR exists to prevent." + +### 5.2 The seam, end to end + +``` +module-uo core +───────── ──── +register(ctx, api) + api.registerEventTriggers([...]) ─────────► registries: shape-check, namespace-check, collide-check + │ +shard event arrives (uoLinkSocket) │ GET /admin/engagement/triggers ──► the rule + template editors + shardIngest → mapper │ + ctx.events.emit('uo.house.idoc_warning', { │ + subject: serial, │ + data: { character, house, location, … }, │ + ownerUserId: ▼ + }) ───────────────────────────────────────► engagement engine + 1. validate payload against the declaration (§4.3) + 2. find enabled rules for this trigger, eval conditions + 3. resolve audience → user ids + 4. per user: check notification_channel_prefs per channel + 5. check engagement_cooldowns (rule, user, subject) + 6. delay_seconds ? enqueue engagement_outbox : deliver now + 7. cancel_on: cancel pending rows for the same subject + │ + ▼ + render template (blocks → HTML + text) + │ + ▼ + DeliveryChannel.deliver → transport → engagement_sends +``` + +**Owner resolution stays in the module.** `module-uo/server/utils/shardPush.js` already turns an +`ownerAcct` into a website user via `shardLinks` — core has no idea what a game account is and must not +learn. The module resolves and passes `ownerUserId`; core never sees `ownerAcct`. + +### 5.3 What this costs the contract + +`MODULE_API_VERSION` 1.6.0 → **1.7.0**. Additions only (`registerEventTriggers`, `ctx.events.emit`, +`ctx.inbox.push`), no removal, no changed signature ⇒ minor by §1.1's table. `module-uo`'s +`coreApi: "^1.3.0"` still resolves, so no module is broken by the bump. + +Knock-on obligations: + +- `client/src/modules/version.js` carries the same number and a test asserts they agree. +- `integration-kit/ci/core-ref.json` pins a website `main` sha and `scripts/checkCoreApi.js` asserts + **equality** with `MODULE_API_VERSION`. **A bump turns the integration kit red on purpose** — that is + the mechanism, not a bug: someone must re-read the chapters and move the pin. Budget a kit PR. +- `docs/website/MODULE_API.md` §1.1, §2.3 and §2.4 need the new members, and §1.1's "1.6.0 has only ever + been on `edge`" paragraph needs correcting (§0.5). + +--- + +## Part 6 — The phased plan + +Same shape as the API v2 router-split plan: grouped, reviewable increments, one acceptance check per +phase, and an explicit note on which guardrails apply. **Every phase carries the standing obligations** +— `npm test --prefix server` green, `npm run swagger` regenerated when a route changes, +`npm run routes:manifest -- --check` clean, `npm run check:modules` clean, a matching `docs/` edit, +Conventional Commits, the AI-disclosure trailer, and a branch cut from a freshly-pulled `main`. + +**Stage A (1–2) is prerequisite. Stage B (3–6) is the engagement system. Stage C (7–8) is the in-app +channel. Stage D (9) is deliverability. Stage E (10–11) is the shard enrichment and runs in parallel +from day one.** + +--- + +### Phase 0 — Design of record ✅ + +This document, landed as `docs/website/ENGAGEMENT.md` with the five settled decisions recorded at the +top. No code. + +**Acceptance:** merged into `docs/`; §7.1's open questions each answered or explicitly deferred before +the phase that depends on them starts — Q5 before Phase 1, Q1/Q3/Q6/Q7 before Phase 2, Q2 before +Phase 4, Q4 before Phase 5b. + +--- + +### Phase 1 — Remove Gmail OAuth2; `DeliveryChannel` + SMTP + +**This phase is a subtraction and a replacement in one PR**, because leaving the OAuth2 flow half-wired +across a release is worse than either end state. + +Delete everything in §1.2a's inventory. Extract `utils/mailer.js` behind the §3.1 interface with `smtp` +as the sole registered transport. Admin → Email becomes a credential form driven by `credentialFields`. +`email_config` gains `transport` / `credential_enc` / `reply_to`, defaulting to `smtp` with no +credentials. All six existing call sites keep their **exact** failure contracts — the contact form's +`mailto` fallback, the invite's copyable-link fallback, the password reset's generic 200, and +`sendTeamNotification`'s never-throws. + +Ships with the cutover safety net §1.2a demands: an admin dashboard warning when the transport is +configured but credential-less, a release note naming the operator action, and the +`smtp.gmail.com:587` + app-password migration path documented as the shortest route for the existing +deployment. + +**Acceptance:** `test/mailer.test.js` and `test/emailConfig.model.test.js` pass (amended only where they +assert OAuth2 specifics); a fresh install with SMTP configured sends every one of the five current +message types; an upgraded install with no SMTP credentials degrades exactly as an unconfigured +deployment does today — contact form falls back to `mailto`, invites surface the link, resets answer 200 +— **and shows the warning**; `grep -r "smtp.gmail.com\|mail.google.com" server/src` returns nothing. +**Guardrails:** swagger regen + `routes:manifest --check` (two routes removed); no-hardcoded-host check +(§3.2 rule 4) — which the deleted `smtp.gmail.com` literal is the first real test of. + +--- + +### Phase 2 — The trigger registry and the variable contract + +`api.registerEventTriggers` + `ctx.events.emit` in `modules/registries.js` and `modules/loader.js`; +`MODULE_API_VERSION` → 1.7.0 on both halves; core registers its own triggers (news, the four Team +events) through `registerCore()`. `npm run engagement:manifest` + the CI `--check`. **No delivery yet** — +emit validates, logs and stops. + +A trigger declaration also carries its **audience ceiling** (G24) — the widest audience a rule may ever +give it — and its `kind` (`event` now, `scheduled` reserved for G25). Both are cheap here and expensive +to retrofit into the rule model later. + +**Acceptance:** core's triggers appear in `GET /admin/engagement/triggers`; a module registering an +un-namespaced trigger fails to load with the holder named; a payload missing a `required` variable +throws in dev and is dropped+logged in prod; **a rule cannot be saved with an audience wider than its +trigger's ceiling**; `engagement-triggers.json` diffs zero in CI. +**Guardrails:** the new manifest `--check` (this is where the "manifest-style guardrail" the brief asks +about belongs); `check:modules` proves core's own trigger ids name no game concept. + +--- + +### Phase 3 — Channel preferences + +`notification_channel_prefs` + the idempotent backfill from `notification_subscriptions`. New +`GET·PUT /auth/me/notifications/channels`. **`/auth/me/notifications/subscriptions` keeps its exact wire +shape** and becomes the push projection — writes fan out to both. + +**Acceptance:** the shipped Android app's flat `{streams:[…]}` PUT still round-trips, including the +empty-array case the app's DTO comment warns about; a per-channel PUT sets `email` without touching +`push`; a fresh user's email mode defaults `off` and push defaults `instant` (§4.5's `defaultMode`). +**Guardrails:** swagger + route manifest; a test pinning the legacy wire shape byte-for-byte. + +--- + +### Phase 4 — The engine: rules, cooldowns, outbox + +`engagement_rules`, `engagement_cooldowns`, `engagement_outbox`, `engagement_sends`, the sweep worker +(`setInterval` + `unref` + `stop`, wired into `server.js` like its five siblings), audience resolution, +condition evaluation, delay and cancellation. Admin → Engagement → Rules. + +**Acceptance:** a trigger fired twice inside `cooldown_seconds` for the same (rule, user, subject) sends +once; the same trigger for a *different* subject sends again; a scheduled row is cancelled by a +`cancel_on` trigger and never sends; a restart mid-window still sends exactly once; a duplicate +`dedupe_key` is a successful no-op. +**Guardrails:** swagger + route manifest; a named test for the multi-house cooldown case (§4.1). + +--- + +### Phase 5 — Templates: the seeded set, then the editor + +Two slices, landing in this order **on purpose** — the seeded set has to exist before the editor, so the +editor is opening something rather than facing a blank page. + +**5a — storage, blocks, renderer, seeds.** `engagement_templates`, the `email.*` block family with +`toText`, the HTML + plain-text renderer, brand-value resolution, and the §4.6.1 seeded set. The five +transactional bodies move out of `mailer.js` into seeded rows and `mailer` renders them. **No editor +yet** — this slice is provably done when the same mail goes out from a template that used to come from a +string literal. + +**5b — the editor.** The §4.6.2 surface: variable palette from the trigger declaration, live preview +from `example` values, side-by-side HTML/text, three preview widths plus dark mode, test send, duplicate. +Built on the existing block/prop-panel machinery, not a second one. + +**Acceptance (5a):** every one of the five current message types renders byte-comparably from its seeded +template; re-running the seeder is a no-op; a seeder bump updates a `customized = 0` row and **skips** a +`customized = 1` one; two deployments with different `BRAND_*` produce differently-branded mail from the +same seed. +**Acceptance (5b):** a template referencing an undeclared variable is refused at save **with the variable +named**; preview renders from examples with no live event; a published template with an empty text part +is refused; a `protected` template cannot be deleted but can be duplicated; a template pinned to an +older `trigger_version` is flagged in the admin list; an interpolated variable containing `