docs(website): record the org lead's answers to Q1/Q3/Q5/Q7, and add Phase 1b #177

Merged
whitlocktech merged 2 commits from docs/engagement-decisions into edge 2026-08-29 00:48:04 +00:00
Member

What & why

Four of ENGAGEMENT.md §7.1's eight open questions were answered by the org lead on 2026-08-28. Two of the answers were larger than the questions — Q1 carries a whole new phase, and Q7 adds a third addition to the module contract. This PR records all four, and the findings that had to be verified before they could be written down.

Docs only. Phase -1 (the edge prep §6.0a made blocking) was also executed while this was written, and is recorded as complete.

The four answers

Q Answer
Q5 — SMTP posture 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.
Q3 — rules as data or code Data, as recommended — 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 the thing that makes "data" safe enough to pick over "code".
Q7 — manual sends No campaigns surface at all. But lists do exist, module-declared. See below.
Q1 — unverified addresses Opt-in only; users.email becomes UNIQUE; the verification gate is an admin setting, on for fresh installs, off for upgrades. See below.

Q2, Q4, Q6 and Q8 remain open, blocking Phases 4, 5b, 2 and 8.

Q7 → §5.1a, audiences

The correction was precise and worth quoting: "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 — thru the uo module; same surface will be exposed to all modules."

So a third contract addition, api.registerAudiences, on the same registration discipline as the triggers. Two rules carry the weight:

  • The resolver returns user ids and nothing else. It is never handed a template, a channel or an address, and cannot enumerate them. A module still cannot send mail (§1.2) and this must not become the back door that changes that.
  • Composition narrows, never widens. Operators may combine declared audiences with and/or/not into a saved segment; the segment takes the tightest ceiling in its tree, re-checked against the trigger's own G24 ceiling. Union-widens is the intuitive implementation and it is the one that lets an operator compose past a staff-only trigger's ceiling. engagement_audience_segments.ceiling is therefore derived at save time, never operator-typed.

Q1 → §0.6 and a new Phase 1b

"Emails need to be unique" reads like a schema tweak. It is not — users.email is VARCHAR(255) NULL with no index and schema.sql:24 says so on purpose. Four findings, each read out of the tree:

  1. The boot-time ALTER would stop the site. Upgrades ride the idempotent block at schema.sql:1409+, run by ensureSchema() on every boot (server.js:66). ADD UNIQUE INDEX against a table holding duplicates throws, and the site does not start. De-duplication must run first, in the same release.
  2. isDuplicateUsername() never inspects which index collidedusers.model.js:24 is ER_DUP_ENTRY || 1062 and nothing more. Unfixed, auth.controller.js:137 answers "That username is already taken." for a duplicate email, and sso.controller.js:197 retries usernames for an email conflict, which can never clear, so it burns PROVISION_MAX_TRIES and fails opaquely. This fix must be in the tree before the index is, not after.
  3. SSO auto-provisioning is the source of the duplicates — and CLAUDE.md's "identities are never auto-provisioned" is stale: provisionSsoPlayer does exactly that when player_registration ∈ {sso, both}, writing profile.email in with emailVerified: Boolean(profile.email) — verified because an address was present, not because the IdP asserted it. That is also why de-dupe is oldest-wins rather than verified-wins: the verified flag is too weak a signal to arbitrate on.
  4. No self-serve email flow exists at all. No route lets a user set or change their own address after signup. A verification gate presupposes a flow to gate, so Phase 1b builds one.

Plus the error-surface consequence: a unique constraint needs a user-facing failure, and the obvious wording makes account existence queryable — against a posture the codebase holds deliberately (passwordReset.controller.js answers a generic 200 "to avoid account enumeration"). Settled: generic message, real reason logged not returned, rate limit kept, and not fed to the bot scorer, so an honest typo on a taken address cannot push a legitimate user toward an IP ban.

Phase 1b lands between 1 and 2 and alone. Two knock-ons: Phase 9 no longer blocks Phase 11 (the verification mechanism moved forward, Phase 9 keeps bounces and suppression), and Phase 2 now follows 1b — an audience resolves to users, and the identity those users are mailed at should be unique before anything resolves over it.

A wrong turn, kept in the doc on purpose

The first commit "corrected" §6.0a to say android-app already had an edge. It did not. git fetch origin does not prune, so a refs/remotes/origin/edge left from the branch deleted after the M12 cutover still resolved — rev-parse succeeded and rev-list --left-right --count returned "1 behind", which reads exactly like a stale-but-present branch. The push reported [new branch] and git ls-remote --heads origin edge confirmed it. The second commit reverts the claim and records the trap: a remote-tracking ref is a cache, not an answer.

How it was tested

Documentation only. Every claim verified against the working trees rather than recalled — schema.sql (users DDL, the ALTER block), users.model.js, auth.controller.js, sso.controller.js, player/account.router.js, server.js. Remote branch existence checked with ls-remote, not remote-tracking refs.

Phase -1 executed alongside: five fast-forwards (module-uo 9 behind, installer 7, servuo-plugins 7, website 5, link 3) and three branches cut from main (android-app, runicgateway.com, Integration-kit). Each fast-forward verified edge == origin/main and origin/edge an ancestor of the result before pushing.

Checklist

  • I have read CONTRIBUTING.md.
  • The change builds and existing tests/checks pass locally. (Docs-only; no build or test surface.)
  • I have added or updated tests/docs where it makes sense.
  • My commits are reasonably scoped with clear messages.

AI-assisted contributions (required)

  • AI tools were used. Tool(s): Claude Code (Opus 5). I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a Co-Authored-By / Assisted-By trailer.

License

  • I agree that my contribution is licensed under this project's license (GNU GPL v3.0 or later), and I have the right to contribute it.
## What & why Four of `ENGAGEMENT.md` §7.1's eight open questions were answered by the org lead on 2026-08-28. Two of the answers were larger than the questions — Q1 carries a whole new phase, and Q7 adds a third addition to the module contract. This PR records all four, and the findings that had to be verified before they could be written down. Docs only. Phase -1 (the `edge` prep §6.0a made blocking) was also executed while this was written, and is recorded as complete. ### The four answers | Q | Answer | | --- | --- | | **Q5** — SMTP posture | 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. | | **Q3** — rules as data or code | **Data**, as recommended — 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 the thing that makes "data" safe enough to pick over "code". | | **Q7** — manual sends | **No campaigns surface at all.** But lists *do* exist, module-declared. See below. | | **Q1** — unverified addresses | Opt-in only; **`users.email` becomes UNIQUE**; the verification gate is an **admin setting**, on for fresh installs, off for upgrades. See below. | Q2, Q4, Q6 and Q8 remain open, blocking Phases 4, 5b, 2 and 8. ### Q7 → §5.1a, audiences The correction was precise and worth quoting: *"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 — thru the uo module; same surface will be exposed to all modules."* So a third contract addition, `api.registerAudiences`, on the same registration discipline as the triggers. Two rules carry the weight: - **The resolver returns user ids and nothing else.** It is never handed a template, a channel or an address, and cannot enumerate them. A module still cannot send mail (§1.2) and this must not become the back door that changes that. - **Composition narrows, never widens.** Operators may combine declared audiences with and/or/not into a saved segment; the segment takes the **tightest** ceiling in its tree, re-checked against the trigger's own G24 ceiling. Union-widens is the intuitive implementation and it is the one that lets an operator compose past a staff-only trigger's ceiling. `engagement_audience_segments.ceiling` is therefore *derived* at save time, never operator-typed. ### Q1 → §0.6 and a new Phase 1b "Emails need to be unique" reads like a schema tweak. It is not — `users.email` is `VARCHAR(255) NULL` with no index and `schema.sql:24` says so on purpose. Four findings, each read out of the tree: 1. **The boot-time `ALTER` would stop the site.** Upgrades ride the idempotent block at `schema.sql:1409+`, run by `ensureSchema()` on *every* boot (`server.js:66`). `ADD UNIQUE INDEX` against a table holding duplicates throws, and the site does not start. De-duplication must run first, in the same release. 2. **`isDuplicateUsername()` never inspects which index collided** — `users.model.js:24` is `ER_DUP_ENTRY || 1062` and nothing more. Unfixed, `auth.controller.js:137` answers *"That username is already taken."* for a duplicate **email**, and `sso.controller.js:197` retries **usernames** for an **email** conflict, which can never clear, so it burns `PROVISION_MAX_TRIES` and fails opaquely. **This fix must be in the tree before the index is**, not after. 3. **SSO auto-provisioning is the source of the duplicates** — and CLAUDE.md's *"identities are never auto-provisioned"* is stale: `provisionSsoPlayer` does exactly that when `player_registration ∈ {sso, both}`, writing `profile.email` in with `emailVerified: Boolean(profile.email)` — verified because an address was *present*, not because the IdP asserted it. That is also why de-dupe is oldest-wins rather than verified-wins: the verified flag is too weak a signal to arbitrate on. 4. **No self-serve email flow exists at all.** No route lets a user set or change their own address after signup. A verification gate presupposes a flow to gate, so Phase 1b builds one. Plus the error-surface consequence: a unique constraint needs a user-facing failure, and the obvious wording makes account existence queryable — against a posture the codebase holds deliberately (`passwordReset.controller.js` answers a generic 200 *"to avoid account enumeration"*). Settled: generic message, real reason logged not returned, rate limit kept, and **not** fed to the bot scorer, so an honest typo on a taken address cannot push a legitimate user toward an IP ban. **Phase 1b** lands between 1 and 2 and alone. Two knock-ons: **Phase 9 no longer blocks Phase 11** (the verification mechanism moved forward, Phase 9 keeps bounces and suppression), and **Phase 2 now follows 1b** — an audience resolves to users, and the identity those users are mailed at should be unique before anything resolves over it. ### A wrong turn, kept in the doc on purpose The first commit "corrected" §6.0a to say `android-app` already had an `edge`. It did not. `git fetch origin` does not prune, so a `refs/remotes/origin/edge` left from the branch deleted after the M12 cutover still resolved — `rev-parse` succeeded and `rev-list --left-right --count` returned "1 behind", which reads exactly like a stale-but-present branch. The push reported `[new branch]` and `git ls-remote --heads origin edge` confirmed it. The second commit reverts the claim and records the trap: **a remote-tracking ref is a cache, not an answer.** ## How it was tested Documentation only. Every claim verified against the working trees rather than recalled — `schema.sql` (users DDL, the ALTER block), `users.model.js`, `auth.controller.js`, `sso.controller.js`, `player/account.router.js`, `server.js`. Remote branch existence checked with `ls-remote`, not remote-tracking refs. Phase -1 executed alongside: five fast-forwards (`module-uo` 9 behind, `installer` 7, `servuo-plugins` 7, `website` 5, `link` 3) and three branches cut from `main` (`android-app`, `runicgateway.com`, `Integration-kit`). Each fast-forward verified `edge == origin/main` and `origin/edge` an ancestor of the result before pushing. ## Checklist - [x] I have read [CONTRIBUTING.md](CONTRIBUTING.md). - [x] The change builds and existing tests/checks pass locally. *(Docs-only; no build or test surface.)* - [x] I have added or updated tests/docs where it makes sense. - [x] My commits are reasonably scoped with clear messages. ## AI-assisted contributions (required) - [x] AI tools were used. Tool(s): `Claude Code (Opus 5)`. I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a `Co-Authored-By` / `Assisted-By` trailer. ## License - [x] I agree that my contribution is licensed under this project's license (**GNU GPL v3.0 or later**), and I have the right to contribute it.
wtclaude added 2 commits 2026-08-29 00:45:28 +00:00
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 <noreply@anthropic.com>
Phase -1 is now executed, and running it disproved the correction made in the
previous commit. android-app's `edge` did not exist: the push reported
`[new branch]`, and `git ls-remote --heads origin edge` confirms it. §6.0a was
right the first time — three repos needed a branch cut, not two.

The cause is worth keeping, because it read as a real measurement. `git fetch
origin` does not prune, so android-app's `refs/remotes/origin/edge` survived the
server-side deletion after the M12 cutover. rev-parse resolved it and
`rev-list --left-right --count` returned "1 behind" — a plausible number for a
stale branch, which is exactly what it looked like. A remote-tracking ref is a
cache, not an answer; ls-remote is.

Records Phase -1 as complete: five fast-forwards (module-uo 9, installer 7,
servuo-plugins 7, website 5, link 3) plus three branches cut from main
(android-app, runicgateway.com, Integration-kit).

Assisted-By: Claude Code (Opus 5)
Co-Authored-By: Claude <noreply@anthropic.com>
whitlocktech approved these changes 2026-08-29 00:47:53 +00:00
whitlocktech merged commit 7a2ad727cb into edge 2026-08-29 00:48:04 +00:00
whitlocktech deleted branch docs/engagement-decisions 2026-08-29 00:48:05 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: RunicGateway/docs#177
No description provided.