From d9abe3d941eab0b96bdb3d90159f34607fda473f Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 31 Aug 2026 02:11:46 -0500 Subject: [PATCH] docs(website): the in-app channel as built (engagement Phase 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ENGAGEMENT.md gains the Phase 7 as-built: the four decisions the org lead settled before any code (in-app defaults to `instant`; the phase takes push's `deliver` and the web preferences screen; the inbox takes `/notifications` and the settings move under it; `ctx.inbox.push` respects a preference where one exists), the block-role mapping that turns a template into a row, and five things the tree contradicted or the build found — including the one only the live rig could see, that staff had no reachable inbox at all. Two earlier passages amended where the phase made them false: Phase 2's "`ctx.inbox.push` throws until Phase 7" and Phase 3's "`inapp` is declared `off`". Both kept as history with the correction beside them. BACKEND_DESIGN.md gains the four inbox routes in the `/auth/me` table and a `user_notifications` entry in the table inventory — the user-scoped dedupe index, why `body` is text rather than the email HTML, and the retention policy. Code: RunicGateway/website#TBD Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 43 +++++++++++ website/ENGAGEMENT.md | 159 +++++++++++++++++++++++++++++++++++++- 2 files changed, 199 insertions(+), 3 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index f2c884a..95bbc9c 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -680,6 +680,45 @@ to the in-code seed whenever the row is absent or its `blocks` will not parse runs, after a restore that dropped the table, or on a row hand-edited in the database. That fallback is what makes it safe for a password-reset mail to depend on this table at all. +### user_notifications — the in-app inbox (engagement phase 7) +| col | type | notes | +|---|---|---| +| id | BIGINT AUTO_INCREMENT PK | | +| user_id | INT NOT NULL FK→users(id) ON DELETE CASCADE | CASCADE, unlike `engagement_sends`: this is content addressed to a person, not an audit of what the deployment sent | +| trigger_id | VARCHAR(96) NOT NULL | denormalized, **no foreign key** — a trigger is declared in code | +| title | VARCHAR(300) NOT NULL | rendered from the template's first `email.heading`; falls back to the projected `title`, then to the key. Truncated rather than refused | +| body | TEXT NULL | the **text** render of the template's remaining blocks. Not the email HTML — see below | +| url | VARCHAR(500) NULL | **site-relative only**, validated with the same character class `pageUrlTemplate` and the engine's `url` variables use. An absolute url on this deployment's own base is reduced to a relative one; anything else is dropped to NULL | +| dedupe_key | VARCHAR(190) NULL | NULL = this item does not dedupe | +| read_at | DATETIME NULL | | +| created_at | DATETIME | | + +`UNIQUE (user_id, dedupe_key)`, `INDEX(user_id, read_at, created_at)`, `INDEX(created_at)`. + +**The unique key is scoped to the USER, and that is deliberately narrower than the outbox's.** +`engagement_outbox` scopes its dedupe to `(rule, user, channel)` because one event legitimately becomes +one row per channel; an inbox has no channel dimension, so two rows for one event would be one item +shown twice. Multiple NULLs are permitted by a UNIQUE index, which is what "does not dedupe" means, and +`INSERT IGNORE` is what makes a replay, a retry and a module writing the same item twice all one no-op. + +**`body` is text, and that is the load-bearing choice rather than a shortcut.** The `email.*` renderer +produces markup built for mail clients — table rows, inline hex colours, a light-only palette declared +with `color-scheme` — which dropped into a page that follows the viewer's theme renders as a pale card +floating in a dark one. `toText` is the same content with none of that, and it is the part the block +contract already promises every block can produce. It also means there is no operator markup on this +surface to sanitize, and no way for one to appear: every renderer treats the column as text. + +**The template maps onto the three columns by block ROLE** (`templates.renderInappByKey`): the first +`email.heading` is the title, the first `email.button` is the url, and everything else is the body. So +an operator editing `inapp.event` in the Phase 5b editor changes what appears in the inbox, which is the +only reason the template exists at all. + +**Retention: `utils/userNotificationsPrune.js`, nightly, READ items only.** Age alone would delete the +evidence for "I was never told", which is the complaint this table answers, and an inbox that quietly +drops unread items is one whose badge means nothing. The horizon is `settings.user_notifications_retain_days` +(default 90), so an operator tightens a busy shard without a deploy — `team_activity`'s posture, in the +worker that file is modelled on. + ### The two block registries — pages and mail (engagement phase 5a) `server/src/blocks/` (the CMS page family) and `server/src/emailBlocks/` (`email.heading`, `email.text`, @@ -1098,6 +1137,10 @@ their own router level, and `/sso/:provider/link` carries `requireAuth` per rout | GET · PUT | `/me/notifications/subscriptions` | cookie / bearer | `{streams:[id]}` on PUT | get / replace own opted-in streams (unknown ids dropped) | | GET · PUT | `/me/notifications/channels` | cookie / bearer | `{prefs:[{id,channel,mode}]}` on PUT | get / update own **per-channel** preferences ([`ENGAGEMENT.md`](ENGAGEMENT.md) §4.5, phase 3). Returns the delivery-channel registry (`email`/`push`/`inapp`, each with `defaultMode`, `supportsDigest`, `modes`) plus one item per subscribable id — the **union** of push streams and event triggers, one namespace (§7.2) — carrying the **effective** mode on each channel that applies to it. A trigger-only id has no `push` toggle; a mode with no stored row reads as that channel’s default, so a client never sees which is which. The PUT is **sparse**: only the `(id, channel)` pairs listed are written and every other pair is untouched, so setting `email` cannot disturb `push`. `off` is a mode, never an omission — which is why this endpoint has no required-empty-array case. Entries naming an unknown id, an inapplicable channel or a mode that channel does not accept are **dropped, not refused**; the full stored state is echoed back. A `push` entry is mirrored into `/me/notifications/subscriptions`, whose wire shape is unchanged | | GET · PUT | `/me/notifications/teams` | cookie / bearer | `{teams:[{teamId,muted,emailMode}]}` on PUT | get / replace own **per-Team** preferences (phase 6, [`TEAMS.md`](TEAMS.md) §6.3). One entry per Team the caller could be notified about — active membership or an active forum grant — plus any Team they already hold a preference for; server-side defaults applied. An entry naming a Team the caller has no access to is **dropped, not refused**: a Team left between loading the screen and saving it is a race, not a client bug. The array is required even when empty (`../android/PLAN.md` §11) | +| GET | `/me/notifications` | cookie / bearer | `?limit&before&unread` | **one page of the caller's in-app inbox** ([`ENGAGEMENT.md`](ENGAGEMENT.md) §4.5 G17, phase 7), newest first. `before` is a **keyset cursor** (the previous page's last id), never an offset: the list gains rows at the top while it is being read. `limit` defaults to 30, capped at 100. Carries `unread`, the count for the whole inbox rather than the page, so a client rendering both a list and a badge cannot show them disagreeing. **No parameter names a user** — the caller is the only account any of these four routes can read | +| GET | `/me/notifications/unread-count` | cookie / bearer | — | `{unread}`. Its own route because it is **polled**: asking "is there anything new" must not make the server assemble a page of bodies to answer with one integer | +| POST | `/me/notifications/:id/read` | cookie / bearer | — | mark one item read. **Idempotent** — the statement carries `read_at IS NULL`, so a second call does not move the stamp. **404 both** when no such item exists and when it belongs to another account: the same answer on purpose, so this cannot be used to ask whether an id is anybody's | +| POST | `/me/notifications/read-all` | cookie / bearer | — | mark the whole inbox read; returns `{ok, changed, unread:0}` | **Role-agnostic self-service (`/auth/me/*`).** The **only** self-service account surface, for every authenticated role, behind `requireAuth` **only** — any active account, never a specific role. A diff --git a/website/ENGAGEMENT.md b/website/ENGAGEMENT.md index e92e33b..37cfb7e 100644 --- a/website/ENGAGEMENT.md +++ b/website/ENGAGEMENT.md @@ -1334,7 +1334,7 @@ change is not complete until `docs/` reflects it" — is the floor; this table i | **5a** Templates ✅ | `website/ENGAGEMENT.md` §4.6 as built · `BACKEND_DESIGN.md` — the `engagement_templates` table, the two block registries, the token grammar, and §7's multipart/subject changes | Landed with the phase | | **5b** The editor | `website/ENGAGEMENT.md` §4.6.2 as built · `BACKEND_DESIGN.md` route table | **`runicgateway.com`**: a new admin docs page for the template editor | | **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 · `website/ENGAGEMENT.md` §4.2b + this phase as built · `BACKEND_DESIGN.md` route table and table inventory | **`runicgateway.com`**: `administration/teams.mdx` notification section. Landed with the phase | -| **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 | +| **7** In-app channel (core+web) ✅ | `website/BACKEND_DESIGN.md` routes + tables (the four inbox routes, `user_notifications`) · `website/ENGAGEMENT.md` this phase as built | **`runicgateway.com`**: `notifications-and-email.mdx` gains the in-app channel. Landed with the phase | | **8** In-app (Android) | `android/PLAN.md` | `android-app/README.md` | | **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` | @@ -1660,6 +1660,7 @@ mails `uo.cheat.detected` to the player it detected. Fewer people is not less ex - `ctx.events.emit` (`utils/engagementEmit.js`) — validate, log, **stop**; throws in dev, drops and logs in prod; the owner is bound by core and never read from the arguments - `ctx.inbox.push` — present and **throws** until Phase 7, the shape 1.6.0 settled on + *(Phase 7 filled it in. Not a version bump: the signature is the one 1.7.0 declared.)* - `config/coreTriggers.js` — core's five, registered through `registerCore()` - `GET /admin/engagement/{triggers,audiences}` — admin-only, served from the registries, no table - `npm run engagement:manifest` (+ `--check` in CI) and the committed `engagement-triggers.json` @@ -1745,7 +1746,9 @@ phases before the registry replaced it. What did *not* land is the behavioural h `transports/index.js` deferred the whole file in Phase 1. Core's three channels are declared, and `inapp` is declared `off` for a reason particular to it — the inbox does not exist until Phase 7, and a default of `instant` would mean every user is opted into a surface with no rows, so the first thing -Phase 7 shipped would be a backlog. +Phase 7 shipped would be a backlog. **Phase 7 changed it to `instant`** once there was a surface to +look at: an inbox item wakes no device and leaves no building, and the backlog this paragraph feared +cannot happen against an empty table. See Phase 7's decision 1. **The sparse PUT is the one place this phase leaves the router's idiom, and it buys two things.** A whole-set body forces a client that only manages email to send every push row back or wipe them. And @@ -2606,7 +2609,7 @@ started writing, which is the column Phase 9's bounce correlation reads. --- -### Phase 7 — The in-app channel (core + web) +### Phase 7 — The in-app channel (core + web) ✅ `user_notifications`, the in-app `DeliveryChannel`, `GET /auth/me/notifications` + mark-read, and the web surface (bell + list). Push tickles gain a `ref` that deep-links into the inbox. @@ -2616,6 +2619,156 @@ no-op; mark-read is idempotent; a user cannot read another user's row (asserted the model); `url` is relative-only, validated by the same character-class rule `pageUrlTemplate` uses. **Guardrails:** swagger + route manifest; the sanitize path for `body`. +#### As built — 7 (2026-08-31) + +The third channel gets behaviour, the oldest one gets a `deliver` at last, and the preferences endpoint +Phase 3 shipped with no surface gets one. **Four decisions were settled by the org lead before any +code**, two of them widening the phase past its own acceptance line. + +##### Decision 1 — `inapp` defaults to `instant`, and it is the only channel that does + +`coreChannels.js` deferred this in as many words: "whether the inbox is opt-out once it is real is a +Phase 7 decision with a live surface to look at." The surface exists now, and the answer is opt-OUT. + +The argument for opt-IN was never about in-app. §7.1 Q1 is standard marketing-email practice and Phase +3's `push` default is about a device somebody is holding; **an inbox item wakes nothing and leaves +nothing** — it is a row on a page the user chose to open, on this deployment, costing one glance. Left +at `off` the channel would ship dead: no rule could reach anybody until every user found a toggle for a +channel they had never seen deliver anything. The backlog Phase 3 worried about cannot happen either — +the table is empty at cutover, rules default to `enabled = 0`, and every rule carries a per-hour +ceiling. + +##### Decision 2 — the phase takes the two pieces its acceptance line omitted + +Two earlier phases assigned work here that Phase 7's own bullets never mention, and both were taken: + +- **`push` gets its `deliver`** (§2603's "the push and in-app channels' `deliver`"). Without it a rule + naming push still finished `failed` in the send log — the oldest sink in the system, unreachable from + the engine. It is the channel that got behaviour last because until the inbox existed there was + nothing for a content-free tickle to point at. +- **The web per-channel preferences screen** (Phase 3's as-built: "the screens are Phase 7 (web) and + Phase 8 (app)"). The endpoint had shipped with no consumer on either platform. + +##### Decision 3 — the inbox takes `/notifications`; the preferences move under it + +`/auth/me/notifications/*` was already the preferences namespace — `streams`, `subscriptions`, +`channels`, `teams` — and `/account/notifications` was already the preferences *page*, with a bell icon +in the portal nav. Content and settings are different kinds of thing, and **the plain word belongs to +the content**: it is what a person means when they say "notifications", and what the bell opens. + +So the inbox is `GET /auth/me/notifications` and the page is `/account/notifications`; the preferences +screen moved to `/account/notifications/settings` and gained its own nav row. Route order is not +incidental and is commented as such: the four named preference sub-paths are declared above, and the one +parameterised path added below them is a **POST** whose `:id` is digits-only, so nothing can shadow +`streams` or `channels`. + +##### Decision 4 — `ctx.inbox.push` respects a preference where one exists + +The rule-less sink has no trigger declaration to project from, no rule to pick a template and no +audience to resolve. It now writes the inbox directly **unless** `triggerId` names a *registered* +trigger and that user's effective `inapp` mode is not `instant`: a toggle somebody switched off must not +be walkable around by the module that owns the trigger behind it. An id nothing has registered has no +toggle on any screen, so there is no preference to protect and the item is written. + +Scoped preferences are deliberately not consulted — a scope is a property of an *event* (`team:12`), and +a caller with no declaration has no scope to name. The engine's path, which does, still applies them. + +##### The block → column mapping, which is the whole of how a template becomes a row + +`user_notifications` has `title` / `body` / `url` where email has a subject and a document. The in-app +renderer (`templates.renderInappByKey`) maps by block **role**: the first `email.heading` is the title, +the first `email.button` is the url, everything else is the body. A second heading or button is ordinary +body content, which is what an operator who added one meant. + +**The body is TEXT, not the email HTML**, and that is load-bearing rather than a shortcut. The `email.*` +renderer produces markup built for mail clients — table rows, inline hex colours, a light-only palette +declared with `color-scheme` — which dropped into a page that follows the viewer's theme renders as a +pale card floating in a dark one. `toText` is the same content with none of that, and it is the part +every block already promises. The consequence worth stating: **there is no operator markup on this +surface to sanitize, and no way for one to appear.** The phase's "sanitize path for `body`" guardrail is +discharged by the column never holding markup in the first place, which is a stronger guarantee than a +sanitizer. + +##### Five things the tree contradicted, or the build found + +- **The shipped `inapp.event` seed named variables nothing supplies.** Phase 5a wrote it before the + channel that renders it existed, declaring `body` and `url` — but a trigger declares domain names + (`teamName`, `threadTitle`) and `projection.project` fills the gaps with the *structural* ones + (`title`, `intro`, `actionUrl`). Every rendering would have produced a title and nothing else. + Renamed to `notify.event`'s vocabulary at **`seedVersion` 2**, which is §4.6.1 property 1 restated for + this channel: a new trigger must render with no authoring at all. +- **The dedupe index is scoped to the USER, which is narrower than the outbox's.** `engagement_outbox` + scopes to `(rule, user, channel)` because one event legitimately becomes one row per channel; an inbox + has no channel dimension, so two rows for one event would be **one item shown twice**. Same family of + defect as the global index Phase 4a found in §4.2a, in the opposite direction. +- **The push tickle's `ref` needed an ordering to be worth anything.** A rule spanning `inapp` and + `push` enqueues two independent rows and the outbox sweeps `ORDER BY due_at, id`, so the ref only + resolves if the in-app row was enqueued first. `engine.liveChannels` now sorts `inapp` ahead of the + rest (`CHANNEL_ORDER`) — an ordering, not a dependency: the ref is a **hint**, null when there is no + row, and the app's contract stays wake-and-pull. +- **There was no retention policy for this table at all**, and neither the outbox nor the send log + bounds it (both hold one row per *delivery*; an inbox item outlives its delivery by design). + `utils/userNotificationsPrune.js` is `teamActivityPrune`'s shape with one policy difference: + **read items only.** Age alone would delete the evidence for "I was never told", which is the + complaint this table answers. The horizon is `settings.user_notifications_retain_days`, default 90. +- **Staff had no reachable inbox, and only the live rig could see it.** `/auth/me/notifications` is + role-agnostic — behind `requireAuth` only, like every `/auth/me` route — so the server, the tests + and the API all agreed a staff member had an inbox. On the web they did not: `RequirePlayer` sends + anyone who is not a player out of `/account` (staff manage their own account under `/admin/account`), + so the bell pointed at a page that redirects. **Signed in as an admin, the feature was unreachable.** + Fixed by mounting the same two components at `/admin/notifications` and + `/admin/notifications/settings`, adding the bell to the admin header, and putting the one mapping in + `client/src/lib/notificationPaths.js` with its own test. One trap inside the fix worth keeping: + `allowedPathsFor` turns an `end: true` nav row into an EXACT match, so marking the admin row exact + left `/admin/notifications/settings` outside the allowlist and bounced staff off their own + preferences screen — the row has to cover its sub-routes. + +##### What an operator and a user actually see + +- **The bell** sits in the public site header and in the player portal's own header, renders nothing + when signed out, and **polls** its badge once a minute — pausing while the tab is hidden and + refreshing the moment it comes back. There is nothing to push over: the site's two SSE streams are + the shard's, neither is per-user, and a third authenticated stream carrying one integer would mean an + open connection per signed-in tab forever. +- **The preferences screen is now a matrix**, not a checkbox list. The push-only stream list it replaced + was a strict subset: `/notifications/channels` already returns every push stream *and* every event + trigger with the effective mode on each channel that applies, so a trigger-only id simply has no push + cell and core never has to explain which kind of id a row is. The two legacy whole-set endpoints are + untouched and are that surface's push projection, so **the shipped Android app keeps its wire shape**. + +##### What was verified + +- **28 new tests**: 23 in `engagementInapp.test.js` (the five acceptance criteria, the role mapping, the + four `ctx.inbox.push` cases, the tickle's exact key set, and the route-level ownership check) and 5 in + `userNotificationsSql.test.js` — a throwaway MariaDB, because three properties here are a *server* + contract rather than a reading of this code: a UNIQUE index admitting many NULLs, `INSERT IGNORE` + reporting `affectedRows = 0` on a duplicate, and `read_at IS NULL` making mark-read idempotent. +- **Two existing tests moved with the behaviour, and both moves are the point.** + `engagementEngine`'s "a channel with no `deliver()` finishes failed" named `inapp` (and `email` before + it) and so was rewritten by every phase that gave a channel behaviour; it now registers a throwaway + channel, because the property was never about a particular one. `notificationChannelPrefs`'s defaults + assertion carries decision 1. +- **Swagger and the route manifest** carry the four new routes, with four new component schemas. +- **A live rig**: MariaDB + a booted server + the real outbox worker + a browser. The walk is where + the staff-reachability defect came from, and it also proved the three things unit tests cannot — + that `liveChannels`' ordering really does put the in-app row first (a rule stored as + `["push","inapp"]` enqueued outbox 2 = inapp before outbox 3 = push, and the tickle carried + `ref: "notification:2"`); that **two rules on one event produce three outbox rows and exactly ONE + inbox item**, with the send log saying "already in this inbox (duplicate dedupe key)" rather than + claiming a second delivery; and that the retention worker drops an aged READ row while leaving an + equally aged UNREAD one. The preferences matrix wrote exactly one row for the one cell that changed. + +**One thing this phase did NOT wire, and it is worth knowing before Phase 11.** `news.post` is a +declared trigger that **nothing emits through the engine** — `coreTriggers.js` says so in as many words +("these declare; nothing here emits yet") and Phase 6 migrated only the four `team.*` ones, so the +admin publish path still fires a raw `pushDispatch.publish` beside the engine rather than through it. +The consequence for this phase: on a real deployment the only in-app items a rule can produce today +come from the four Team triggers. Wiring the news emitter is a one-line `ctx.events.emit`-shaped change +that belongs with whoever owns that decision, not smuggled into the channel's own phase. + +**Still later phases':** the app's inbox screen and the tickle → pull → inbox path (Phase 8), and the +suppression list, which this channel has no equivalent of — there is no address to suppress. + --- ### Phase 8 — The in-app channel (Android) -- 2.49.1