RunicGateway/docs
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(website): the engagement engine as built (Phase 4a)

Browse Source 
Companion to website#170. Section 6.0b's assignment for Phase 4 - ENGAGEMENT.md's
as-built and BACKEND_DESIGN.md's table inventory - plus the two corrections
building it forced on this document's own design sections.

ENGAGEMENT.md
  - Phase 4 is split 4a / 4b, with what each owes.
  - Section 7.1 Q2 and Q4 answered, so seven of eight are settled and only Q8
    (android CI) is open.
  - The as-built: the gate order and why two of its placements are load-bearing,
    the segment rule the design never stated (not is legal only inside an and,
    and contributes no ceiling), dormancy three ways and why audience_segment_id
    has no foreign key, the conditions grammar's two fail-closed properties, and
    why emit does not await the engine.
  - Section 4.1's cooldown statement and section 4.2a's dedupe index are
    corrected in place, so the design sections stop teaching the two defects.

BACKEND_DESIGN.md
  - Five new tables in the schema inventory, each with the reasoning a reader
    would otherwise have to reconstruct: why subject_key is in the primary key,
    why the dedupe index is scoped, why the send log survives an account
    deletion and is not a second address book, and why a rule points at a
    segment without a foreign key doing it.

No route table changes - Phase 4a adds no routes.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
claude
2026-08-29 08:07:42 -05:00
parent 713e6fa6c8
commit deaf491dc5
2 changed files with 316 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

213
website/ENGAGEMENT.md
View File

@@ -1,12 +1,14 @@
# The Engagement System — findings and plan
**Status:** design of record. **Phases 1, 1a, 1b, 2 and 3 are built** (Phase 1: website#165 +
**Status:** design of record. **Phases 1, 1a, 1b, 2, 3 and 4a are built** (Phase 1: website#165 +
docs#178, with website#164 as its prerequisite; Phase 1a: website#166 + docs#179; Phase 1b:
website#167 + docs#180; Phase 2: website#168 + docs#181); everything from Phase 4 on is still design.
The scope decisions below are settled; **six of the eight questions in §7.1 are answered** — Q1, Q3,
Q5 and Q7 on 2026-08-28, and Q6 on 2026-08-29 at the start of Phase 2, which also settled §7.2's
namespace question. Q1's answer added a whole phase (**Phase 1b**, unique email addresses). **Q2, Q4
and Q8** remain open and block Phases 4, 5b and 8 respectively. Per CLAUDE.md § Conventions, no
website#167 + docs#180; Phase 2: website#168 + docs#181; Phase 3: website#169 + docs#182; Phase 4a:
website#170 + docs#183); everything from Phase 4b on is still design.
The scope decisions below are settled; **seven of the eight questions in §7.1 are answered** — Q1, Q3,
Q5 and Q7 on 2026-08-28, Q6 on 2026-08-29 at the start of Phase 2 (which also settled §7.2's
namespace question), and **Q2 and Q4 on 2026-08-29 at the start of Phase 4**. Q1's answer added a
whole phase (**Phase 1b**, unique email addresses); Q4's answer and the phase's size split **Phase 4
into 4a and 4b**. **Q8** remains open and blocks Phase 8. 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
@@ -651,8 +653,13 @@ drops three of them. The module supplies `subject` on emit; core stores it opaqu
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.
**The check must not be a read-then-write**, or two concurrent emits both see an expired cooldown and
both send. The obvious single statement — `INSERT … ON DUPLICATE KEY UPDATE` with the interval guard in
the assignments, reading the answer out of `affectedRows` — **does not work against this codebase's
pool**, and Phase 4a is where that was found: the mariadb connector defaults `foundRows: true`, so a
no-op update reports 1 rather than 0 and every cooldown passes. What ships instead is a guarded UPDATE
(the interval in a WHERE clause, where a row either matches or does not) falling back to an
`INSERT IGNORE` for the first fire. See Phase 4a's as-built for the statements and the races.
### 4.2 Scheduling — a queue for delay, and deliberately no queue for digest
@@ -679,7 +686,10 @@ CREATE TABLE IF NOT EXISTS engagement_outbox (
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),
-- SCOPED, not global. One event legitimately becomes one row per (rule, user,
-- channel); a global unique index would admit the first recipient's row and
-- silently ignore every other. Corrected in Phase 4a — see its as-built.
UNIQUE KEY uq_engo_dedupe (rule_id, user_id, channel, 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;
@@ -691,8 +701,10 @@ Three things this buys that a straight send does not:
- **`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.
- **`dedupe_key` UNIQUE per (rule, user, channel) 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. The scope matters as much as the constraint: a dedupe key names the *event*,
and the event fans out to every recipient of every channel of every matching rule.
`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`.
@@ -1281,7 +1293,8 @@ change is not complete until `docs/` reflects it" — is the floor; this table i
| **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`, `registerAudiences`), the dormant-rule note (landed as §6.8) · `website/ENGAGEMENT.md` §4.3 and §5.1a kept true · `BACKEND_DESIGN.md` route table | **Both deferred to the Phase 13 cutover window, deliberately — see Phase 2's as-built.** `Integration-kit`'s `ci/core-ref.json` pins a **`main`** sha, so the equality check stays green (and must stay green) for the whole `edge` period; `runicgateway.com`'s `checkFacts.mjs` *fetches* from `main`, so setting `platform.json.moduleApi` → 1.7.0 now would turn that repo red immediately |
| **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 | — |
| **4a** Engine | `website/ENGAGEMENT.md` (rules/cooldown/outbox as built, and the two §4 defects it corrects) · `BACKEND_DESIGN.md` table inventory | — |
| **4b** Rules screen | `website/BACKEND_DESIGN.md` route table · `website/ENGAGEMENT.md` §5.1a composition UI | — |
| **5a/5b** Templates + editor | `website/ENGAGEMENT.md` §4.6 · a template-authoring section in `BACKEND_DESIGN.md` or its own doc | **`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 | **`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 |
@@ -1742,15 +1755,163 @@ see something a preference actually governs.
### 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.
**Split into 4a and 4b** at the start of the phase, on the same argument that split Phase 5: the half
that first makes this system capable of sending is worth reviewing without a React screen in the same
diff, and the ceiling arithmetic that decides who a rule may reach is worth reading on its own.
#### 4a — the engine (server only) ✅
`engagement_rules`, `engagement_audience_segments`, `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, and the
save-path validation the admin surface will call. **No HTTP surface at all** — provably done when a
fired trigger produces an outbox row and a send-log entry with no UI in the picture.
#### 4b — the admin surface
Admin → Engagement → Rules, the §5.1a segment composition UI, and the routes underneath them. Q4's
answer places it in **its own top-level nav group** (below), so 4b also creates the group that
Triggers, Templates and the send log join in Phase 5.
**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).
**Guardrails:** swagger + route manifest (4b — 4a adds no routes); a named test for the multi-house
cooldown case (§4.1).
---
#### As built — 4a (2026-08-29)
**Three decisions were settled by the org lead before any code, and two of them are §7.1 questions
this phase was blocked on.**
| | Question | Decision |
|---|---|---|
| **Q2** | multi-instance: `SKIP LOCKED`, or document single-instance | **Neither, exactly**: a compare-and-set claim — `UPDATE … SET status='sending' WHERE id=? AND status='scheduled'`, the winner being whoever the server reports `affectedRows = 1` to. It is what §4.2a's ENUM was already shaped for (nothing else needs a `sending` state), it needs no open transaction and no MariaDB version floor, and it delivers Q2's intent |
| **Q4** | where the engagement admin surface lives | **its own top-level nav group**, "Engagement", beside Content / Moderation / System — Rules now, Triggers / Templates / Send Log in Phase 5. Email Delivery stays a section of Settings for now |
| — | one PR or two | **4a / 4b**, as above |
**What Q2's answer does and does not buy.** It makes the *outbox* safe for two app instances. It does
not make the deployment multi-instance: `announceWorker`, `teamDigestWorker`, `teamForumUploadSweep`
and `teamActivityPrune` are all still written for one, and widening them is not this phase's scope.
What it buys is that the one table that will carry mail is ready for the day it is, which is cheap now
and expensive after mail has doubled once.
**Two defects in this document's own §4, both found by building it.**
1. **§4.2a's `UNIQUE KEY uq_engo_dedupe (dedupe_key)` was a data-loss bug, not a style question.** A
dedupe key names the EVENT — "house 0x4001 entered IDOC" — and one event legitimately becomes many
outbox rows: an audience of fifty users is fifty rows, a rule spanning email and in-app doubles
that, and a second rule on the same trigger doubles it again. Under a *global* unique index the
first of those inserts wins and every other one is silently ignored, so ninety-nine recipients are
dropped by the mechanism that exists to stop a replayed event becoming a second mail. Shipped as
**`UNIQUE (rule_id, user_id, channel, dedupe_key)`**, which keeps exactly the replay guarantee and
nothing more. A test asserts one key fans out to six rows.
2. **§4.1's single `INSERT … ON DUPLICATE KEY UPDATE` cooldown claim does not work against this
codebase's pool**, and the way it fails is silent. Its answer is read out of `affectedRows` on the
usual contract — 1 inserted, 2 updated-and-changed, **0 for a duplicate key whose update changed
nothing**, that 0 being "still cooling". **The mariadb Node connector defaults `foundRows: true`**,
which makes `affectedRows` report rows *matched* rather than rows *changed*, and `utils/db.js` does
not override it. Under that pool the no-op returns 1 and is indistinguishable from a fresh insert:
**every cooldown passes, always.** Shipped as two statements instead, each of which is its own
atomic decision and neither of which asks `affectedRows` to mean two things:
```sql
-- 1. claim by moving the row, guarded in a WHERE clause where a row either matches or does not
UPDATE engagement_cooldowns SET last_fired_at = ?, fire_count = fire_count + 1
WHERE rule_id = ? AND user_id = ? AND subject_key = ?
AND last_fired_at <= ? - INTERVAL ? SECOND;
-- 2. matched nothing? then the row is absent or cooling; INSERT IGNORE separates the two
INSERT IGNORE INTO engagement_cooldowns (rule_id, user_id, subject_key, last_fired_at, fire_count)
VALUES (?, ?, ?, ?, 1);
```
Still race-free, and each race resolves the right way: two concurrent first fires both fall to the
INSERT and the primary key picks one; two concurrent fires after expiry serialise on the row lock
and the second re-evaluates its guard against the committed `last_fired_at`.
**The second defect is the reason this phase has a second test file.** `engagementEngine.test.js`
stubs the five tables and runs the engine's logic against in-memory stand-ins, which is right for
everything the engine *decides* — and it was **green against the broken cooldown claim**, because a
stub can only agree with whoever wrote it, and the same misreading produced both. The statements whose
correctness is a *server* contract now run against a real MariaDB in `engagementEngineSql.test.js`,
which creates a throwaway database, drops it, and **skips when there is none** so CI stays green
without one. The general lesson: a stub is a fine stand-in for a table and a poor one for a protocol.
**The gate order is the design.** Enabled rules → conditions → audience → **ceiling re-check** →
per-channel preference → per-rule hourly ceiling → cooldown → deduped enqueue. Two of those placements
are load-bearing:
- **The G24 ceiling is re-checked at SEND time, not only at save.** The save path already ran the same
`ceilings.permits`, so the only way this can fail is the case it exists for — a module upgrade that
*narrows* its trigger's declaration underneath a rule saved when it was wider. Without it, a rule
written against yesterday's declaration keeps reaching yesterday's population forever. This is the
second call site §5.1a promised, not a second implementation.
- **The hourly ceiling is checked before the cooldown**, because the ceiling is about the rule and the
cooldown is about one recipient. A rule that has hit its ceiling should not also burn every
recipient's cooldown slot on sends that never happen.
**Segments (§5.1a) as built, with one rule the design did not state.** `not` is legal **only as a
child of `and`**. A complement needs a universe, and the only one available that does not widen is
the set its siblings produced: `A AND NOT B` is "A, less B", which is what an operator wants and
cannot be composed into a broadcast. A bare `NOT B` — or `A OR NOT B` — would have to mean "everyone
except…", which is a way to build the whole deployment out of one narrow audience and is precisely the
widening rule 3 forbids. It is refused at save with that sentence.
The other half of that: **a `not` contributes no ceiling to the meet.** Excluding people cannot widen
who an expression reaches, so folding the excluded audience's ceiling in would refuse safe segments —
`members AND NOT staff` would hit `meet('members','staff') = null` and be rejected even though it
reaches strictly fewer people than `members` alone.
**Dormancy, three ways, and none of them deletes anything** (§7.3, §5.1a rule 4). A rule naming an
unregistered trigger, a rule whose channel is gone, and a rule whose segment was deleted are all
*listed, flagged and left alone*. In particular `engagement_rules.audience_segment_id` deliberately
carries **no foreign key**: `ON DELETE CASCADE` would delete an operator's rules and `ON DELETE SET
NULL` would silently fall the rule back to its plain `audience` column — and that fallback reaches a
*different set of people*, which is the exact failure §5.1a rule 4 exists to prevent. Deleting a
segment that a rule still uses is refused in the model, with the count.
**Conditions are a small closed grammar**, not an expression language: and/or/not over comparisons of
one *declared* variable against a literal, every operator renderable as a dropdown, bounded in list
length and nesting depth because the tree comes out of a JSON column an admin can write and is walked
on the emit path. Two properties worth keeping:
- **An absent variable makes every comparison false, including `ne`.** "Not equal to IDOC" reads as
satisfied by nothing at all, and treating it that way would fire a rule on every event that omits an
optional variable. `present` / `absent` are the honest way to ask.
- **A tree that no longer parses evaluates false**, never "no conditions". Failing closed stops the
mail; failing open mails everyone the rule could ever reach.
**`ctx.events.emit` does not await the engine.** It is called from inside a game-event handler, and
the caller's job is to say the event happened — not to wait on rule lookups, audience resolution and a
dozen inserts to find out whether it is allowed to carry on. That is the same argument the C# side's
`Emit()` makes about the Core thread. `dispatch` catches everything internally and never rejects. The
consequence a caller must know: **`emit` returns before the outbox rows exist**, so a test that wants
the delivery decision calls `engine.dispatch` directly.
**Nothing is delivered, and that is visible rather than pretended.** A channel's `deliver` arrives with
email in Phase 6 and the in-app inbox in Phase 7. Until then the worker claims the row, finds no
`deliver`, finishes it `failed`, and writes a send-log row saying so in as many words. Recording
`sent` would be a lie in the one table whose entire purpose is answering "did they get it"; leaving the
row `scheduled` would mean an IDOC warning queued today arriving three weeks later on the deploy that
first shipped a mailer. On a real deployment the path is unreachable anyway — core seeds no rules and
`enabled` defaults to 0, so nothing enqueues until 4b's screen exists and an operator uses it.
**Three smaller things the phase settled:**
- **`everyone` and `authenticated` resolve identically.** A signed-out visitor has no address, no
device and no inbox, so the widest set the engine can deliver to is the active user table. The
lattice still distinguishes them — a trigger ceilinged `everyone` permits an `authenticated` rule
and not the reverse — and only the *resolution* coincides.
- **A plain `members` audience with no segment reaches nobody.** `members` is the ceiling for "a
module-declared list"; without a segment there is no list, and core knows no game vocabulary with
which to guess. Inert and visible, rather than quietly falling back to something wider.
- **Every audience is filtered through `users.status = 'active'`, including a module's.** A module's
resolver returns ids over its own store and has no notion of account status; a banned account must
not be mailable by a module returning its id.
---
@@ -2009,7 +2170,7 @@ day it ships.
## Part 7 — Open questions and forward-compat notes
### 7.1 Questions for the org lead — six answered, three still open
### 7.1 Questions for the org lead — seven answered, one still open
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
@@ -2025,15 +2186,23 @@ day it ships.
*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.
2. ✅ **ANSWERED — multi-instance.** Neither of the two the question offered, and the third is
better than both: the outbox sweep **claims each row with a compare-and-set** —
`UPDATE … SET status='sending' WHERE id=? AND status='scheduled'` — and the instance the server
reports `affectedRows = 1` to owns it. §4.2a's status ENUM already carried a `sending` state that
nothing else needed, so this is what the schema was shaped for; it needs no open transaction (which
nothing else in this codebase's workers does) and no MariaDB version floor. It makes the **outbox**
safe for two app instances and does not, on its own, make the deployment multi-instance — the four
existing workers are still written for one. Built in Phase 4a.
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.
4. ✅ **ANSWERED — the engagement admin surface gets its own top-level nav group**, "Engagement",
beside Content / Moderation / System. Four screens is too much to bury: Settings is already one long
page of sections, and a send log is a paged table rather than a settings section. Rules lands with
Phase 4b; Triggers, Templates and the send log join it in Phase 5. **Email Delivery stays a section
of Settings** for now — moving it is not part of either phase.
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