From 3d93b3865c80595de247f9b96770162f51e02901 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Fri, 28 Aug 2026 19:40:49 -0500 Subject: [PATCH] docs(website): record the org lead's answers to Q1/Q3/Q5/Q7, and add Phase 1b MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four of the eight open questions in ENGAGEMENT.md §7.1 were answered on 2026-08-28. Q1's answer turned out to carry a whole phase with it. - Q5: document all three SMTP postures, lead with a relay, name Gmail-app-password as the migration path off OAuth2. - Q3: rules stay operator-editable data, but `enabled` defaults to 0 and every rule carries a hard per-hour send ceiling. Adds max_sends_per_hour to engagement_rules — the ceiling is what makes "data" safe to choose over "code". - Q7: no campaigns surface at all. Lists DO exist, but only module-declared and powered by module data, on a surface core exposes to every module. Operators may compose them; composition must NARROW, never widen. Adds §5.1a, api.registerAudiences and engagement_audience_segments. - Q1: opt-in only, users.email becomes UNIQUE, and the verification gate is an admin setting (on for fresh installs, off for upgrades). New §0.6 records why the UNIQUE index is not a one-line ALTER, verified in the tree rather than assumed: - ensureSchema() runs the ALTER block on every boot, so ADD UNIQUE INDEX against a table holding duplicates stops the site from starting. - isDuplicateUsername() tests only ER_DUP_ENTRY/1062 and never which index collided, so register would answer "that username is already taken" for a duplicate email, and provisionSsoPlayer would retry usernames for an email conflict until it exhausts PROVISION_MAX_TRIES and fails opaquely. - SSO auto-provisioning manufactures those duplicates and marks addresses verified merely for existing — which is also why dedupe is oldest-wins rather than verified-wins. CLAUDE.md's "identities are never auto-provisioned" is stale. - No route lets a user change their own address, so a verification gate has no flow to gate; Phase 1b builds one. New Phase 1b sequences the fix before the index, dedupes oldest-wins with an admin report, and keeps the collision error generic, rate-limited and out of the bot scorer. Phase 9 loses the verification flow to it and therefore no longer blocks Phase 11. Also corrects §6.0a: android-app DOES have an edge (1 behind main), so Phase -1 is six fast-forwards and two branch creations, not five and three. Assisted-By: Claude Code (Opus 5) Co-Authored-By: Claude --- website/ENGAGEMENT.md | 301 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 261 insertions(+), 40 deletions(-) diff --git a/website/ENGAGEMENT.md b/website/ENGAGEMENT.md index 696d34f..9a16cbe 100644 --- a/website/ENGAGEMENT.md +++ b/website/ENGAGEMENT.md @@ -1,12 +1,14 @@ # 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 eight 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. +**Status:** design of record for the next workstream. No code written yet. The scope decisions below are +settled; **four of the eight questions in §7.1 were answered by the org lead on 2026-08-28** — Q1, Q3, Q5 +and Q7, and Q1's answer added a whole phase (**Phase 1b**, unique email addresses). Q2, Q4, Q6 and +Q8 remain open and block Phases 4, 5b, 2 and 8 respectively. Per CLAUDE.md § Conventions, no implementation +starts without the org lead's approval of the phase it belongs to. **Branching:** every phase lands on **`edge`** in its repo; `main` is touched once, by the cutover -(Phase 13). §6.0a records the blocking precondition — every existing `edge` is stale and three repos -have none. +(Phase 13). §6.0a records the blocking precondition — six `edge` branches are stale and two repos have +none. **Scope decisions, settled by the org lead (2026-08-28):** @@ -19,9 +21,25 @@ have none. 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. +**Four further decisions, settled 2026-08-28** (recorded in full at §7.1, with the findings that shaped +them in §0.6): + +6. **Engagement mail is opt-in only, and `users.email` becomes UNIQUE.** Standard marketing-email practice + applies: explicit opt-in, working unsubscribe, suppression. Uniqueness is not a detail — it is + **Phase 1b**, because the column is nullable-and-duplicated by design today and three code paths break + the moment an index is added. Whether an *unverified* address may receive opt-in mail is an **admin + setting**, defaulting **on for fresh installs and off for upgrades**. +7. **There is no campaigns surface.** No operator-authored send screen, no free-form list building. An + admin's expressive power lives in trigger conditions and rules. +8. **Audiences are module-declared and operator-composable.** A module registers named, queryable + audiences over *its own* data (`uo.team.members`, `uo.governors`); core exposes the same registration + surface to every module and learns no game vocabulary. An operator may compose declared audiences with + and/or/not into a saved segment — and the composed result is still bounded by the trigger's G24 + audience ceiling. See §5.1a. + --- -## Part 0 — Five findings that contradict the brief +## Part 0 — Six findings that contradict the brief Stated up front because the rest of the document is shaped by them. @@ -129,6 +147,54 @@ So the engagement additions take **1.7.0** — additions only (`api.registerEven --- +### 0.6 A UNIQUE email is not a one-line ALTER — it breaks three paths and can stop a boot + +Decision 6 (opt-in only, unique addresses) reads like a schema tweak. It is not. `users.email` is +`VARCHAR(255) NULL` with **no** unique index, and `schema.sql:24` says so deliberately: *"Optional +contact email (players). Not unique — SSO emails may repeat."* Adding the index touches registration, +SSO provisioning and the upgrade path. All four findings below were read out of the tree on 2026-08-28. + +**1. The boot-time `ALTER` is how schema reaches a deployment, and it would fail loudly.** Upgrades ride +the idempotent `ALTER TABLE … IF NOT EXISTS` block at `schema.sql:1409+`, executed by `ensureSchema()` +on **every** boot (`server/src/server.js:66`). `ADD UNIQUE INDEX` against a table that already holds +duplicate addresses errors, `ensureSchema()` throws, and **the site does not start**. A de-duplication +step must run before the index, in the same release — see Phase 1b. + +**2. `isDuplicateUsername()` does not inspect which index collided.** `users.model.js:24` is +`err.code === 'ER_DUP_ENTRY' || err.errno === 1062` and nothing more. Two callers misread an email +collision as a username collision the instant the index exists: + +| Site | Today | After `UNIQUE(email)`, unfixed | +| --- | --- | --- | +| `auth.controller.js:137` (register) | 409 *"That username is already taken."* on a genuine username race | Same message for a duplicate **email** — wrong, and it misattributes the conflict to the one field the user did not collide on | +| `sso.controller.js:197` (`provisionSsoPlayer`) | Retries the next username suffix on collision | Retries **usernames** for an **email** conflict, which can never clear; burns `PROVISION_MAX_TRIES` and returns `null`, so SSO sign-up fails opaquely with the log blaming usernames | + +The fix is to distinguish the constraint (read the index name off the driver error) before Phase 1b adds +the index — not after. + +**3. SSO auto-provisioning is the source of the duplicates, and CLAUDE.md is stale about it.** CLAUDE.md +states *"identities are never auto-provisioned"*. `provisionSsoPlayer` (`sso.controller.js:197`) does +exactly that whenever `player_registration ∈ {sso, both}`, writing `profile.email` straight into +`users.email`. It also sets `emailVerified: Boolean(profile.email)` — **verified because an address was +present, not because the IdP asserted `email_verified`**. That matters twice over: it manufactures the +duplicates Phase 1b must clean up, and it makes `email_verified` too weak a signal to arbitrate *which* +duplicate wins (which is why §7.1 Q1's answer is oldest-wins, not verified-wins). + +**4. There is no self-serve email flow at all.** No route lets a user set or change their own address +after signup — `router/v1/player/account.router.js` has none, and `users.model.js:72`'s `update()` is +reached only by admin user management. An address is captured once, at registration or SSO provisioning, +and is thereafter unchangeable by its owner. A verification gate presupposes a change-and-verify flow, +so Phase 1b builds one; it is not an add-on to an existing screen. + +**One consequence for the error surface.** A unique constraint needs a user-facing failure, and the +obvious wording (*"that email is already registered"*) makes account existence queryable — a step back +from a posture the codebase holds deliberately elsewhere (`passwordReset.controller.js` answers a generic +200 *"to avoid account enumeration"*). §7.1 Q1 settles it: the message stays generic, the real reason is +logged not returned, the endpoint stays rate-limited, and the failure is **not** fed to the bot scorer — +an honest typo on a taken address must not push a legitimate user toward an IP ban. + +--- + ## Part 1 — Current-state map ### 1.1 Notification system, end to end @@ -746,6 +812,11 @@ CREATE TABLE IF NOT EXISTS engagement_rules ( 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', + audience_segment_id INT NULL, -- a composed segment (§5.1a); NULL = the plain audience above + -- §7.1 Q3: the hard stop that makes operator-editable rules safe to choose over + -- code-registered ones. Counted in engagement_sends, enforced before the outbox + -- row is written, never overridable from the rule editor beyond this column. + max_sends_per_hour INT NOT NULL DEFAULT 100, 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] @@ -759,6 +830,22 @@ CREATE TABLE IF NOT EXISTS engagement_rules ( INDEX idx_engr_trigger (trigger_id, enabled) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- §5.1a: an operator-composed segment over module-declared audiences. Stored as a +-- boolean tree of audience ids + params; `ceiling` is DERIVED at save time as the +-- NARROWEST ceiling in the tree and re-checked against the trigger's own ceiling, +-- so composition can never widen. It is a column rather than a runtime computation +-- so an audit can read what a rule was allowed to reach without re-resolving it. +CREATE TABLE IF NOT EXISTS engagement_audience_segments ( + id INT AUTO_INCREMENT PRIMARY KEY, + name VARCHAR(160) NOT NULL, + expression JSON NOT NULL, -- { op: 'and'|'or'|'not', nodes: [...] | { audienceId, params } } + ceiling VARCHAR(32) NOT NULL, -- derived, never operator-typed + 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_engseg_user FOREIGN KEY (updated_by) REFERENCES users(id) ON DELETE SET NULL +) 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, @@ -921,7 +1008,7 @@ supply markup. ## Part 5 — The module registration mechanism -### 5.1 Two additions to the contract, both modelled on what already works +### 5.1 Three additions to the contract, all modelled on what already works ```js // api — what the module registers (MODULE_API.md §2.4). Modelled on @@ -936,6 +1023,10 @@ 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 }) + +// api — the audiences a module can resolve over its own data (decision 8). +// Same registration discipline as the triggers above; see §5.1a. +api.registerAudiences([{ id, label, description, ceiling, resolve }]) ``` **Why a new surface rather than extending `registerNotificationStreams`.** A stream entry is a @@ -956,6 +1047,46 @@ streams and the Discord leg. That is not ceremony — `registries.js`'s header s 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.1a Audiences — module-declared, operator-composable *(decision 8)* + +The org lead's correction to Q7 is precise and worth stating exactly: **there is no campaign surface, but +lists exist — powered by game data, through the module, on a surface every module shares.** "Team X's +members" and "the governors" are legitimate audiences; "everyone who opened the last mail" is not, and +nothing here builds it. + +```js +api.registerAudiences([{ + id: 'team.members', // namespaced() prefixes it → 'uo.team.members' + label: 'Members of a team', + params: [{ id: 'teamId', type: 'int', required: true }], + ceiling: 'members', // the widest this audience can EVER resolve to (G24) + resolve: async (params, ctx) => [/* user ids */], +}]) +``` + +**Four rules, each of which exists because of something already in the tree:** + +1. **Core learns no game vocabulary.** Core never knows what a governor is; it knows an id, a label and + a `resolve` it may call. This is the same boundary `registerNotificationStreams` holds, and + `check:modules` already proves core's own ids name no game concept. +2. **The resolver returns user ids and nothing else.** It is not handed a template, a channel or an + address, and it cannot enumerate them — a module still cannot send mail (§1.2), and this must not + become the back door that lets it. The engine maps ids to addresses on core's side, after + preferences, suppression and the verification gate. +3. **A composed segment is bounded by the *narrowest* ceiling it contains, not the widest.** Operators + may combine declared audiences with and/or/not into a saved segment. That is real power and it is the + part with a security edge: composition must never *widen*. `A OR B` takes the tighter of the two + ceilings, and the result is still checked against the trigger's own G24 ceiling before a rule using it + can be saved. Union-widens is the intuitive implementation and it is the wrong one. +4. **An audience whose module is uninstalled goes dormant, exactly as a rule does** (§7.3). It resolves + to the empty set and the rule referring to it shows as dormant — never an error, never auto-deleted, + never a silent send to a *different* set of people because the id stopped resolving. + +**Where it lands.** The registration surface and the ceiling arithmetic belong in **Phase 2**, with the +trigger declaration — G24's reasoning applies unchanged, and both are cheap now and expensive to retrofit +into a rule model that already has rows in it. The composition UI belongs with the rules screen in +**Phase 4**. `module-uo`'s first real audiences come in **Phase 11**. + ### 5.2 The seam, end to end ``` @@ -992,8 +1123,8 @@ 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 +`MODULE_API_VERSION` 1.6.0 → **1.7.0**. Additions only (`registerEventTriggers`, `registerAudiences`, +`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: @@ -1017,7 +1148,7 @@ regenerated when a route changes, `npm run routes:manifest -- --check` clean, `n clean, **the documentation edits §6.0b assigns it**, Conventional Commits, the AI-disclosure trailer, and a branch cut from a freshly-pulled base. -**Stage A (1–2) is prerequisite. Stage B (3–6) is the engagement system. Stage C (7–8) is the in-app +**Stage A (1–1b–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. Stage F (12–13) is the public site and the cutover.** @@ -1037,8 +1168,14 @@ different days. `main` must never hold a half-applied set of them. refreshed after merging. **Fast-forward each `edge` to `main` before the first phase PR** — it is lossless (0 ahead), and skipping it means the cutover diff carries stale content or conflicts that have nothing to do with this workstream. -2. **Three repos have no `edge` at all** and need one cut from `main`: `android-app` (its M12 branch - was deleted after that cutover), `runicgateway.com`, and `Integration-kit`. +2. **Two repos have no `edge` at all** and need one cut from `main`: `runicgateway.com` and + `Integration-kit`. *(Corrected 2026-08-28: `android-app` **does** have an `edge`, 1 behind `main`, so + it is a fast-forward like the rest. The claim that its M12 branch was deleted after that cutover was + wrong.)* + +**Phase -1 as actually measured, 2026-08-28.** Six fast-forwards — `module-uo` 9 behind, `installer` 7, +`servuo-plugins` 7, `website` 5, `link` 3, `android-app` 1 — and two branch creations. `docs`' `edge` is +already done and is 3 ahead of `main`. Every one is 0 ahead, so all eight are lossless. **Android CI does not run on `edge`.** `android-app/.gitea/workflows/pr-checks.yml` triggers only on PRs into `main`, so every Phase 8 PR lands with **zero CI** and the cutover is the first real run. That @@ -1058,6 +1195,7 @@ change is not complete until `docs/` reflects it" — is the floor; this table i | Phase | `docs/` | Other repos | | --- | --- | --- | | **1** Remove Gmail OAuth2, SMTP | `website/BACKEND_DESIGN.md` §7 **rewritten** (not amended — it documents Gmail OAuth2 as *the* mechanism); route tables lose `/admin/email/connect/*` | `website/README.md` + `.env.example` wherever they point at Connect Gmail · **`runicgateway.com`**: `notifications-and-email.mdx` (its "There is no SMTP option" aside is now false), `configuration.mdx:62`, `troubleshooting.mdx:101`, `system-architecture.mdx:117` · a release note | +| **1b** Unique email | `website/BACKEND_DESIGN.md` — the `users` table (the "not unique" note is now false), the new change/verify routes, and the de-dupe migration as an operator-visible upgrade step | `website/README.md` upgrade notes · a release note naming the admin report and the verification-gate default | | **2** Trigger registry | `website/MODULE_API.md` §1.1 (**1.7.0** + correct the stale "1.6.0 has only ever been on `edge`" paragraph), §2.3 (`ctx.events`, `ctx.inbox`), §2.4 (`registerEventTriggers`), §7.3's dormant-rule note · `website/ENGAGEMENT.md` §4.3 kept true | `Integration-kit`: `ci/core-ref.json` re-pinned (the equality check goes red **on purpose**) + chapter 2 gains a "registering a trigger" section · **`runicgateway.com`**: `platform.json.moduleApi` → 1.7.0 | | **3** Channel preferences | `website/BACKEND_DESIGN.md` route table · `android/PLAN.md` §11 | — | | **4** Engine | `website/ENGAGEMENT.md` (rules/cooldown/outbox as built) · `BACKEND_DESIGN.md` table inventory | — | @@ -1065,7 +1203,7 @@ change is not complete until `docs/` reflects it" — is the floor; this table i | **6** Email channel + Teams migration | `website/TEAMS.md` §6.3/§6.4 **rewritten** — the Team pipeline it describes no longer exists as its own thing | **`runicgateway.com`**: `administration/teams.mdx` notification section | | **7** In-app channel (core+web) | `website/BACKEND_DESIGN.md` routes + tables · `website/ENGAGEMENT.md` | **`runicgateway.com`**: `notifications-and-email.mdx` gains the in-app channel | | **8** In-app (Android) | `android/PLAN.md` | `android-app/README.md` | -| **9** Deliverability | `website/BACKEND_DESIGN.md` §7 · a suppression/bounce operator section | **`runicgateway.com`**: `troubleshooting.mdx` gains bounce/suppression · **`PLAY_DATA_SAFETY.md` + `/privacy`** — see Phase 12 | +| **9** Deliverability | `website/BACKEND_DESIGN.md` §7 · a suppression/bounce operator section (the verification flow is Phase 1b's) | **`runicgateway.com`**: `troubleshooting.mdx` gains bounce/suppression · **`PLAY_DATA_SAFETY.md` + `/privacy`** — see Phase 12 | | **10** Protocol bump | `link/INTEGRATION.md` §Housing (table + example) · `link/PLAN.md` §5/§7 · a `link/v5.md` if the bump earns its own design doc, as v3 and v4 did | `servuo-plugins/overlay.toml` · **`runicgateway.com`**: `platform.json.protocol` → 5, `bundle.*`, `architecture/protocol-versions.mdx` | | **11** module-uo triggers | `modules/uo/API.md` · `modules/uo/README.md` | `module-uo/README.md` | | **12** Public site | — | **`runicgateway.com`**, in full — see the phase | @@ -1084,8 +1222,9 @@ This document, landed as `docs/website/ENGAGEMENT.md` with the five settled deci 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. +the phase that depends on them starts. **Q1, Q3, Q5 and Q7 were answered on 2026-08-28** (§7.1), which +unblocked Phases 1, 2 and 4 and added Phase 1b. Still outstanding: **Q6 before Phase 2**, **Q2 before +Phase 4**, **Q4 before Phase 5b**, **Q8 before Phase 8**. --- @@ -1116,6 +1255,55 @@ deployment does today — contact form falls back to `mailto`, invites surface t --- +### Phase 1b — Unique, changeable, verifiable email addresses *(decision 6)* + +**Lands alone, between 1 and 2, and before any engagement mail exists.** It touches registration, SSO +provisioning and the boot-time schema path — three security-sensitive surfaces — and retrofitting +uniqueness *after* a send log and a suppression list hold rows is strictly worse than doing it now. +§0.6 is the finding this phase discharges. + +**Four pieces, in this order within the PR:** + +1. **Distinguish the constraint before adding one.** Replace `isDuplicateUsername()`'s bare + `ER_DUP_ENTRY`/`1062` test with a check that reads the violated index off the driver error, and give + its two callers (`auth.controller.js:137`, `sso.controller.js:197`) separate branches. **This must be + in the tree before the index is**, or the register path starts lying and SSO sign-up starts failing + opaquely the moment the ALTER runs. +2. **De-duplicate, then index.** A migration step that runs *before* the `ALTER`: for each duplicated + address, the **earliest-created** account keeps it; every later duplicate has `email` set to `NULL` + and `email_verified` to `0`. Multiple `NULL`s are legal under a UNIQUE index, so nobody loses an + account and nothing cascades. The affected accounts are written to an admin-visible report — *who* + was cleared and *what* address they lost — because they are exactly the users who must be contacted. + Then `ALTER TABLE users ADD UNIQUE INDEX IF NOT EXISTS uq_users_email (email)`, pinned to a + case-insensitive collation for the same reason `username` was (`schema.sql:24`): `Foo@x.com` and + `foo@x.com` are one mailbox everywhere that matters, and folding must happen in the index rather than + in application code that can be bypassed. +3. **A self-serve change-and-verify flow**, which does not exist today (§0.6 finding 4). Set/change + address, a signed time-boxed verification link, `email_verified` set only on link use. SSO's + `emailVerified: Boolean(profile.email)` is corrected at the same time to honour the IdP's actual + `email_verified` claim rather than the mere presence of an address. +4. **The verification gate as an admin setting** — `on` for fresh installs, `off` for upgrades, so the + live deployment does not silently stop mailing its existing opted-in users on the day it upgrades. + The asymmetry is deliberate and is the same lesson as G22: a safe default must not be applied + retroactively to a running system without telling anyone. + +**The error surface stays anti-enumeration.** A collision returns a generic failure, the real reason is +logged and not returned, the endpoint keeps its rate limit, and the failure is **not** scored by the bot +detector — a legitimate user typing a colleague's address must not be pushed toward an IP ban for it. + +**Acceptance:** a DB seeded with three accounts sharing an address boots clean, keeps the oldest, nulls +two, and lists both in the admin report; registering with a taken address returns the generic failure, +logs the specific one, increments no bot score, and does **not** say "username"; SSO sign-up with an +address already held by another account fails with a distinguishable reason rather than exhausting +`PROVISION_MAX_TRIES`; a user can change their address and it stays `email_verified = 0` until the link +is used; `Foo@x.com` collides with `foo@x.com`; an upgraded install has the gate `off` and a fresh one +`on`. +**Guardrails:** swagger regen + `routes:manifest --check` (the change/verify routes are new); the +migration is idempotent and re-running `ensureSchema()` is a no-op; **no destructive DDL** — the +de-dupe nulls a column, it never deletes a row. + +--- + ### Phase 2 — The trigger registry and the variable contract `api.registerEventTriggers` + `ctx.events.emit` in `modules/registries.js` and `modules/loader.js`; @@ -1127,8 +1315,13 @@ A trigger declaration also carries its **audience ceiling** (G24) — the widest give it — and its `kind` (`event` now, `scheduled` reserved for G25). Both are cheap here and expensive to retrofit into the rule model later. +Phase 2 also lands `api.registerAudiences` and the ceiling arithmetic (§5.1a) — the same registration +discipline, and the same "cheap now, expensive later" argument G24 makes for the trigger ceiling. + **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 +un-namespaced trigger or audience fails to load with the holder named; **an `A OR B` composition takes +the narrower of the two ceilings, not the wider**; an audience whose module is uninstalled resolves +empty and shows dormant rather than erroring; 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 @@ -1238,16 +1431,20 @@ branch get **no CI** — the cutover PR is the first real run. Plan for that. --- -### Phase 9 — Deliverability: suppression, bounces, verification +### Phase 9 — Deliverability: suppression and bounces -`engagement_suppressions`, bounce/complaint capture per transport (SMTP has none — this is where the -API-based providers earn their place), and an email-verification flow so `users.email_verified` finally -has a writer for self-registered addresses. Policy decision needed: **may an unverified address receive -engagement mail at all?** (Recommendation: transactional yes, engagement no.) +`engagement_suppressions` and bounce/complaint capture per transport (SMTP has none — this is where the +API-based providers earn their place). + +**The verification flow is no longer part of this phase.** §7.1 Q1's answer moved it forward into +**Phase 1b**, along with the admin gate setting that decides whether an unverified address is excluded. +Phase 9 therefore *consumes* `email_verified` rather than introducing a writer for it, and — the reason +this reshuffle is worth it — **Phase 9 no longer blocks Phase 11.** The first real rule can ship on the +verification mechanism 1b already built, with bounces following. **Acceptance:** a suppressed address is skipped with `status='suppressed'` in `engagement_sends` and no -transport call; a hard bounce suppresses the address; an unverified address is excluded from engagement -rules but still receives password resets. +transport call; a hard bounce suppresses the address; with the Phase 1b gate `on`, an unverified address +is excluded from engagement rules but still receives password resets; with it `off`, it receives both. --- @@ -1379,7 +1576,7 @@ is then fast-forwarded to `main` again so the next workstream starts from a clea Phase -1 fast-forward every edge to main; create edge in android-app, runicgateway.com and Integration-kit ← blocking, §6.0a -Stage A 1 ── 2 +Stage A 1 ── 1b ── 2 Stage B └─ 3 ── 4 ── 5a ── 5b ── 6 Stage C └─ 7 ── 8 (8 = app-store cadence) Stage D └─ 9 @@ -1393,7 +1590,9 @@ Stage F 12 ── 13 (12 written before **Phase -1 is blocking and takes minutes.** Every `edge` is 0 ahead / 3–16 behind `main` (§6.0a), so the fast-forward is lossless; skipping it means the cutover diff carries other workstreams' leftovers. -Phases 1, 2 and 10 can start immediately and in parallel. **Phase 1 and Phase 6 are the two that touch +Phases 1, 1b and 10 can start immediately and in parallel; **Phase 2 now follows 1b** rather than 1, +because a trigger's audience resolves to users and the identity those users are mailed at should be +unique and verifiable before anything resolves an audience over it. **Phase 1 and Phase 6 are the two that touch mail people actually receive** and should each land alone: Phase 1 because it can silently stop email for the live deployment (§1.2a), Phase 6 because it rewrites the pipeline behind notifications going out today. Both get the local rig exercised before merge, not only tests. @@ -1412,31 +1611,53 @@ day it ships. ## Part 7 — Open questions and forward-compat notes -### 7.1 Questions for the org lead (not blocking Phase 1 or 2) +### 7.1 Questions for the org lead — five answered 2026-08-28, four still open -1. **May unverified addresses receive engagement mail?** Recommendation: no — transactional only. This - decides whether Phase 9 blocks Phase 11 or merely follows it. +1. ✅ **ANSWERED — may unverified addresses receive engagement mail?** *"Emails need to be unique and + verification blocking sending is an admin setting."* Combined with the opt-in answer, this settles + three things at once and creates **Phase 1b**: + - **Opt-in only**, following standard marketing-email practice — explicit consent, working + unsubscribe, suppression. This matches the `team_notification_prefs` pattern already in use. + - **`users.email` becomes UNIQUE.** Not a schema tweak — see §0.6 for the three code paths it breaks + and the boot it can stop, and Phase 1b for the work. Duplicates are resolved **oldest-wins**: the + earliest account keeps the address, later ones are nulled and listed in an admin report. + - **The verification gate is an admin setting**, default **on for fresh installs, off for upgrades**, + so a running deployment does not silently stop mailing its opted-in users at cutover. + - The collision error stays **generic and anti-enumeration**, rate-limited but **not bot-scored**. + + *Consequence for ordering:* Phase 9 no longer blocks Phase 11 — the verification *mechanism* moves + forward into 1b, and Phase 9 keeps only bounces and suppression. 2. **Multi-instance.** Do we commit to single-instance (status quo) and document it, or add `SELECT … FOR UPDATE SKIP LOCKED` to the outbox sweep in Phase 4? Recommendation: add it in Phase 4 — it is cheap there and expensive to retrofit after mail has doubled once. -3. **Rules as data vs. rules as code.** The plan makes rules operator-editable rows. The alternative is - rules registered in code by whoever owns the trigger, with only enable/disable in the DB. Data is more - flexible; code is far easier to test and impossible to misconfigure into a mail storm. - Recommendation: data, but with `enabled` defaulting to 0 and a hard per-rule hourly send ceiling. +3. ✅ **ANSWERED — rules as data vs. rules as code.** Data, as recommended: rules are operator-editable + rows, `enabled` defaults to `0`, and every rule carries a **hard per-rule hourly send ceiling**. The + ceiling is not a nicety — it is the thing that keeps a misconfigured rule from becoming a mail storm, + and it is what makes "data" safe enough to choose over "code". Phase 4 builds both. 4. **Does the engagement admin surface belong under Admin → Settings, or its own top-level section?** It is three screens (Triggers, Rules, Templates) plus a send log. -5. **Which SMTP posture is the documented default for operators?** Their own mail server, a relay - (Mailgun/SES/Postmark) over SMTP, or Gmail-with-an-app-password. Recommendation: document all three, - lead with a relay, and name Gmail-app-password explicitly as the migration path off OAuth2 for the - existing deployment (§1.2a). +5. ✅ **ANSWERED — which SMTP posture is the documented default?** Document all three; **lead with a + relay** (Mailgun/SES/Postmark); name Gmail-with-an-app-password (`smtp.gmail.com:587`) explicitly as + the migration path off OAuth2 for the existing deployment (§1.2a). Phase 1 owes all three in + `runicgateway.com`'s `notifications-and-email.mdx` as well as `BACKEND_DESIGN.md` §7. 6. **Time-based triggers (G25) — design now, build when?** Recommendation: design the declaration in Phase 2 so `kind: 'scheduled'` exists in the contract, build the evaluator after Phase 9. The lifecycle uses in §8.5 are the highest-value non-game triggers on the list and the first thing anyone will ask for after the IDOC mail works. -7. **Manual/operator-authored sends** (§8.5, last paragraph) — in or out? It is the one campaign-shaped - feature, it is genuinely useful, and it is also the thin end of the wedge the brief rules out. - Recommendation: in, but as a `scheduled` trigger with an explicit staff audience and no list - building — never a separate "campaigns" surface. +7. ✅ **ANSWERED — manual/operator-authored sends.** *"There is no campaign in the normal sense of email + marketing. But admins can create all sorts of trigger conditions"*, and separately: *"lists can be + built if they are powered by game data — say team X members or governors or whatever — thru the uo + module; same surface will be exposed to all modules."* + + So: **no campaigns surface, no operator-authored send screen, no free-form list building.** An + admin's expressive power lives in trigger conditions and rules. `kind: 'scheduled'` therefore stays in + the Phase 2 contract as an admin-definable *condition*, not as a campaign. + + **Lists do exist, module-declared.** A module registers named audiences over its own data on a surface + core exposes to every module, and an operator may compose them with and/or/not into a saved segment. + Composition must **narrow, never widen** — the segment takes the tightest ceiling it contains and is + still checked against the trigger's G24 ceiling. §5.1a is the design; Phase 2 owns the surface, + Phase 4 the composition UI, Phase 11 `module-uo`'s first real audiences. 8. **Android CI on `edge`** (§6.0a). `android-app/.gitea/workflows/pr-checks.yml` triggers only on PRs into `main`, so Phase 8 lands with zero CI and Phase 13 is its first real build — as happened to all nine M12 phase PRs. Fix the trigger as Phase 8's first commit, or accept it deliberately?