From 221c5a9c9ef07e05170b09e043cd9e06045aa483 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 31 Aug 2026 10:47:50 -0500 Subject: [PATCH] =?UTF-8?q?docs(engagement):=20Phase=209=20as=20built=20?= =?UTF-8?q?=E2=80=94=20deliverability,=20suppression=20and=20bounces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records what Phase 9 shipped (website#176) and the four decisions the org lead settled before any of it: mechanism plus SMTP's own synchronous refusal rather than an API transport; suppression scoped to engagement rules only; an `address_masked` column added to §4.5's DDL; and the verification gate applied at enqueue rather than at delivery. The correction the phase's own text needed: "SMTP has none" is too strong. SMTP has no asynchronous bounce feed, but a single-recipient send refused at RCPT TO throws synchronously with the reply code intact, and mailer.js was already catching that and discarding it. The defect worth not repeating: `PERMANENT_CODES` is not a bounce classifier. It answers "is retrying pointless?" and contains EAUTH, so suppressing on it would have emptied the mailing list the first time an SMTP password expired. Also records the one thing only the live rig could find — `engagement_sends` has carried a `bounced` status since §4.5 and nothing had ever written it, so the Send Log's Bounced filter matched nothing — and answers §7.1 Q1's narrower half. - ENGAGEMENT.md: Phase 9 "As built", the §4.5 DDL, Q1's narrower half, status header - BACKEND_DESIGN.md: §3 `engagement_suppressions`, and §7's Deliverability section Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 98 ++++++++++++++++++++++++++- website/ENGAGEMENT.md | 138 +++++++++++++++++++++++++++++++++++--- 2 files changed, 227 insertions(+), 9 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 95bbc9c..8d905e6 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -574,7 +574,8 @@ cooldown passes, always. See `ENGAGEMENT.md` Phase 4a. | trigger_id | VARCHAR(96) NOT NULL | denormalized; survives a rule edit | | user_id | INT NOT NULL FK→users(id) ON DELETE CASCADE | | | channel | VARCHAR(32) NOT NULL | VARCHAR, never ENUM: the channel set is data, and a module must not require an ALTER | -| subject_key | VARCHAR(190) NOT NULL DEFAULT '' | what a COOLDOWN counts, from the trigger's declared `subjectKey`. A display string is fine here: it is only ever compared with itself | +| subject_key | VARCHAR(190) NOT NULL DEFAULT '' | what a COOLDOWN counts, from the trigger's declared `subjectKey`. A display string is fine here: it is only ever compared with itself | + | scope_key | VARCHAR(190) NULL | what a PREFERENCE and an UNSUBSCRIBE are keyed on (engagement phase 6), e.g. `team:12`. Deliberately **not** `subject_key`: an unsubscribe token is signed over this and sits in a mailbox for months, so it has to be a stable identifier — signing over a display name orphans every link the first time somebody renames a Team. NULL means an unscoped event; `''` is reserved for "deployment-wide" in `engagement_digest_state` | | payload | JSON NOT NULL | the declared variables, snapshotted at emit | | dedupe_key | VARCHAR(190) NULL | the emitter's replay guard; NULL never collides | @@ -623,6 +624,35 @@ a rule's budget and mute it. appear here, and neither do they appear in the engagement log lines, which carry variable *names* and counts only. +Phase 9 gave two of those statuses their first writers. `suppressed` means the address was on the +suppression list and **no transport call was made**; `bounced` means one was, and the mailbox does +not exist. `complained` still has none — it needs a provider feedback loop, which SMTP has not got. + +### engagement_suppressions — addresses we have stopped mailing (engagement phase 9) +| col | type | notes | +|---|---|---| +| address_hash | CHAR(64) NOT NULL PK | sha256 of the **lower-cased, trimmed** address | +| address_masked | VARCHAR(190) NULL | `d***@example.com`. Phase 9's one addition to the planned DDL | +| channel | VARCHAR(32) NOT NULL DEFAULT 'email' | | +| reason | ENUM('bounce','complaint','manual','unverified') | | +| detail | VARCHAR(500) NULL | e.g. `hard bounce: 5.1.1` | +| created_by | INT NULL FK→users(id) ON DELETE SET NULL | the admin, for a manual row; **NULL for an automatic one**, which is what separates the two | +| created_at | DATETIME | | + +`INDEX(created_at)`, `INDEX(reason, created_at)` — the screen's two orderings. + +G16. **Keyed on the address, not the user**, and after Phase 1b made addresses unique that is a +choice rather than a workaround: a bounce arrives as an address, it does not know which account was +behind it, and it stays true after that account changed its address or was deleted. + +Writes are `INSERT IGNORE`, so **the first reason an address was suppressed is the one that +survives** — an address that hard-bounced in March and was manually re-added in June still reads +`bounce`, because that is the fact explaining why the mail stopped. An upsert would let the most +recent write overwrite the diagnosis. + +`address_masked` exists because a hash-only table cannot be operated; the reasoning and the routes +are in §7's *Deliverability* subsection. + ### engagement_digest_state — how far each digest has got (engagement phase 6) | col | type | notes | |---|---|---| @@ -1601,6 +1631,72 @@ credentials. stops**. The admin dashboard warns whenever the deprecated Gmail token is present and no replacement credential is; see [`UPGRADE_NOTES.md`](UPGRADE_NOTES.md). +### Deliverability: suppression, bounces and the verification gate *(engagement phase 9)* + +Engagement Phase 9 ([`ENGAGEMENT.md`](ENGAGEMENT.md) Phase 9). Two mechanisms decide that a person +who is *in* a rule's audience does not get the mail, and they are deliberately at different points +in the pipeline. + +**`engagement_suppressions` — checked at DELIVERY.** Keyed on `address_hash` (sha256 of the +lower-cased address), because a bounce arrives as an address and stays true after the account behind +it changed its address or was deleted. An outbox row can sit through a rule's `delay_seconds` grace +window and an address can bounce inside it, so the only correct check is the one taken immediately +before the transport call — which is also what produces the `status='suppressed'` row in +`engagement_sends` with no transport call at all. + +**The verification gate — applied at ENQUEUE.** With the `email_verification_required` setting on +(seeded in Phase 1b: `on` for a fresh install, `off` for an upgrade), an unverified address is +excluded before an outbox row is written. It hangs off a channel's optional **`eligible(userIds)`** +registration rather than living in the engine: being unverified is an *email* fact, and a rule +spanning email and in-app must still reach that person's inbox. Only `email` declares one. The +excluded count comes back so the admin reach preview reports it instead of quietly promising a +number the engine will not deliver. + +**Scope: engagement rules only.** Password resets, invites, verification mails and the contact form +still attempt to a suppressed or unverified address. This is the posture `passwordReset.controller.js` +already took — user-initiated mail must not be blocked by a background system's opinion, and one +reset to a dead mailbox is not a reputation problem, whereas a rule mailing thousands of people +weekly is. + +**What may write a `bounce` row is narrower than "the send failed".** `src/engagement/bounceClassify.js` +is the only judge, and it is deliberately **not** `mailer.PERMANENT_CODES` — that set answers "is +retrying pointless?" and contains `EAUTH` and `554`, so reusing it would mean one stale SMTP password +suppressing every address the worker touched, silently. The classifier reads the **RFC 3463 enhanced +status** first (`5.1.1`, `5.1.2`, `5.1.3`, `5.1.6`, `5.1.10`, `5.2.1` suppress; `5.3.x`, `5.5.x` and +`5.7.x` never do, being about the server or our standing with it), and falls back — only for `550`, +`551` and `553`, and only past a veto list — to a phrase match. **Anything it is unsure about is not +suppressed:** a false negative costs one retry next month, a false positive costs a person who +silently stops hearing from the deployment. + +SMTP has no *asynchronous* bounce or complaint feed — that is where an API-based provider would earn +its place — but a single-recipient send refused at `RCPT TO` throws synchronously with the reply +code intact, which is the highest-value signal there is and is what this reads. `sendNotification` +therefore returns an `smtp: { code, responseCode, response }` triple alongside its classification; +`retry` and `detail` cannot answer "was this the recipient's fault", since `550 5.1.1` and +`550 5.7.1` are an identical `retry: false`. + +**Statuses.** `engagement_sends.status` gains two real writers: `suppressed` (declined to try) and +`bounced` (tried, the mailbox does not exist). `engagement_outbox.status` records `bounced` as +`failed` — its ENUM has no such value and, from the queue's point of view, a bounced row is one that +finished unsuccessfully. `complained` still has no writer: it needs a provider feedback loop. + +**Routes** (all `adminOnly`, under `/api/v1/admin/engagement`): + +| Route | Notes | +| --- | --- | +| `GET /suppressions` | Paged, filterable by `reason` / `channel` / `search`, plus unfiltered `byReason` totals | +| `POST /suppressions` | `reason` is forced to `manual` — an admin typing an address is not evidence of a bounce. An address already listed answers 200 with `created: false`, not 409 | +| `DELETE /suppressions` | The only way out of the list. The address goes in the **body**, not the path: a path parameter lands in the access log, the browser history and every proxy in front of the deployment | + +**Neither route ever returns `address_hash`**, the same rule `GET /sends` follows: a sha256 of every +address on the deployment, handed to a browser, is an offline dictionary attack. What the list +returns is `address_masked` — `d***@example.com` — which Phase 9 added to §4.5's DDL because a +hash-only table cannot be operated: an operator has to be able to see a whole domain refusing mail +and to let back in somebody who fixed their mailbox. The domain survives intact for the first; the +local part is destroyed rather than shortened, so the column can never be read back as an address +book. The consequence is that **lifting a suppression needs the full address typed in** — the screen +genuinely does not have it, which is the privacy design working rather than a rough edge. + --- ## 7.5 Logging & observability diff --git a/website/ENGAGEMENT.md b/website/ENGAGEMENT.md index bc91526..b83f0be 100644 --- a/website/ENGAGEMENT.md +++ b/website/ENGAGEMENT.md @@ -1,12 +1,13 @@ # The Engagement System — findings and plan -**Status:** design of record. **Phases 1, 1a, 1b, 2, 3, 4a, 4b, 5a, 5b, 6, 7 and 8 are built** +**Status:** design of record. **Phases 1, 1a, 1b, 2, 3, 4a, 4b, 5a, 5b, 6, 7, 8 and 9 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; Phase 3: website#169 + docs#182; Phase 4a: website#170 + docs#183; Phase 4b: website#171 + docs#184; Phase 5a: website#172 + docs#185; Phase 5b: website#173 + docs#186 + runicgateway.com#22; Phase 6: website#174 + docs#187 + runicgateway.com#23; Phase 7: website#175 + docs#188 + runicgateway.com#24; Phase 8: the Android -half, Android-app#42 + docs#190); everything from Phase 9 on is still design. The scope decisions +half, Android-app#42 + docs#190; Phase 9: website#176 + docs#191); everything from Phase 10 on is +still design. The scope decisions below are settled; **eight of the nine 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), **Q2 and Q4 on 2026-08-29 at the start of Phase 4**, and **Q8 on 2026-08-31 at the start of Phase @@ -945,13 +946,30 @@ CREATE TABLE IF NOT EXISTS engagement_sends ( INDEX idx_engs_user (user_id, created_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; --- G16. Keyed on the ADDRESS, not the user: users.email is not unique. +-- G16. Keyed on the ADDRESS, not the user. That was written when users.email was +-- not unique; after Phase 1b it stays, for a better reason: a bounce arrives as +-- an ADDRESS, does not know which account was behind it, and stays true after +-- that account changed its address or was deleted. +-- +-- **`address_masked` and `created_by` are Phase 9's additions to this DDL.** The +-- hash-only table cannot be operated: an operator reading sha256 digests cannot +-- tell three typos from a whole domain refusing mail, and un-suppressing somebody +-- who fixed their mailbox is the one action the table must support. The domain +-- survives so a domain-wide failure is visible; the local part is DESTROYED +-- rather than shortened, so the column can never be read back as an address book. +-- `created_by` is what separates a row an admin typed from one the outbox worker +-- wrote (NULL). CREATE TABLE IF NOT EXISTS engagement_suppressions ( - address_hash CHAR(64) NOT NULL PRIMARY KEY, -- sha256 of the lowercased address - channel VARCHAR(32) NOT NULL DEFAULT 'email', - reason ENUM('bounce','complaint','manual','unverified') NOT NULL, - detail VARCHAR(500) NULL, - created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP + address_hash CHAR(64) NOT NULL PRIMARY KEY, -- sha256 of the lowercased address + address_masked VARCHAR(190) NULL, -- d***@example.com; never the local part + channel VARCHAR(32) NOT NULL DEFAULT 'email', + reason ENUM('bounce','complaint','manual','unverified') NOT NULL, + detail VARCHAR(500) NULL, + created_by INT NULL, -- the admin, for a manual row; NULL if automatic + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_engsup_user FOREIGN KEY (created_by) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_engsup_created (created_at), + INDEX idx_engsup_reason (reason, created_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; -- G17: the in-app inbox. Core, game-agnostic, content-carrying. @@ -2883,6 +2901,103 @@ verification mechanism 1b already built, with bounces following. 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. +#### As built (2026-08-31) + +**Four decisions, settled by the org lead before any code:** + +1. **Mechanism plus SMTP's real signal; no API transport.** The phase's own text says "SMTP has none — + this is where the API-based providers earn their place", and that is too strong. SMTP has no + *asynchronous* bounce or complaint feed, but a single-recipient send refused at `RCPT TO` throws + synchronously with the reply code intact — the highest-value deliverability signal there is, and + `mailer.js` was already catching it as a `PERMANENT_CODE` and throwing it away. So this phase reads + it. A transport MAY declare a bounce handler; none does, and **no webhook route ships** — a route + with no producer is §7.1 Q9's problem in a different costume. +2. **Suppression scopes to engagement rules only.** Password resets, invites, verification mail and + the contact form still attempt. This is the posture `passwordReset.controller.js` already stated + for the verification gate, and the argument carries: user-initiated mail must not be blocked by a + background system's opinion, and one reset to a dead mailbox is not a reputation problem. +3. **`address_masked` is added to §4.5's DDL.** The hash-only table cannot be operated — an operator + staring at sha256 digests cannot tell three typos from a whole domain refusing mail, and + un-suppressing somebody who fixed their mailbox is the one action the table must support. +4. **The verification gate filters at ENQUEUE, not at delivery.** + +**The defect this phase exists to have avoided: `PERMANENT_CODES` is not a bounce classifier.** +The obvious implementation is "the mailer already tells us a failure is terminal, so suppress on +that". `mailer.PERMANENT_CODES` is `{550, 553, 554, EENVELOPE, EAUTH}`, and it answers a different +question — *is retrying pointless?* `EAUTH` is the operator's password being wrong and `554` is a +relay-wide policy refusal; neither says anything about the recipient. Under that implementation **one +stale SMTP credential suppresses every address the outbox worker touches**, with a clean send log, no +warning, and a mailing list that has to be rebuilt by hand. So `bounceClassify.js` is its own judge: + +- the **RFC 3463 enhanced status** decides on its own where there is one — `5.1.1`, `5.1.2`, `5.1.3`, + `5.1.6`, `5.1.10` and `5.2.1` suppress, and `5.3.x`, `5.5.x` and `5.7.x` explicitly never do, being + about the server or about our standing with it; +- without one, a phrase match applies **only** after `550`/`551`/`553` has already narrowed the + failure to the recipient address, and only past a veto list — `552` and `554` are excluded from even + that, because a full mailbox gets emptied and "transaction failed" is what a relay says when it does + not want to say why; +- **anything it is unsure about is not suppressed.** A false negative costs one retry next month; a + false positive costs a person who silently stops hearing from the deployment and cannot find out. + +`sendNotification` now returns an `smtp: { code, responseCode, response }` triple so this is +answerable at all: `retry` and `detail` cannot distinguish `550 5.1.1` from `550 5.7.1`, which are an +identical `retry: false` and mean completely different things. + +**The two mechanisms sit at different points, and the split is the design.** A suppression can appear +inside a rule's `delay_seconds` grace window, so the only correct check is the one taken immediately +before the transport call — which is also what produces the `status='suppressed'` row with no +transport call that the acceptance line asks for. Being unverified is a *standing* property, stable +across that window, so excluding at delivery would write an outbox row purely to throw it away — and +on a deployment that upgraded before verifying anybody, one rule firing would write thousands of +`suppressed` rows nobody can read. + +**The gate is a channel hook, not an engine branch.** It hangs off a new optional +`registerDeliveryChannel({ eligible })`, and `email` is the only channel that declares one. Both +alternatives were wrong in a way the build made obvious: filtering the shared audience before the +per-channel loop silences the wrong sink — **a rule spanning email and in-app must still put an item +in an unverified user's inbox**, since being unverified is a reason not to mail somebody and no reason +at all to hide their notifications — and an `if (channel === 'email')` in `engine.js` puts one +channel's rule inside the generic engine. Its excluded counts flow into `summary.ineligible` and into +the admin reach preview, which until now reported an audience size that was never the number of people +who would get a mail. + +**Both new checks fail OPEN**, and the `try/catch` in `eligible` is load-bearing rather than habit: +`applyRule` awaits it *before* the per-user loop, so an uncaught throw abandons the whole rule for +every channel it names — a rule that silently sent nothing, with a clean log and an empty outbox. That +is G22's shape again, and the recoverable mistake is mail going out. + +**The live rig found the one defect the stubs could not.** Against a real MariaDB, a real SMTP +conversation (mailpit) and the real engine and worker, a hard bounce was recorded as `status='failed'` +— honest, but `engagement_sends.status` has carried **`bounced`** since §4.5 and nothing had ever +written it, so the Send Log's "Bounced" filter matched nothing and always would have. It is now a +distinct outcome, because "the relay would not take this" and "this mailbox does not exist" send an +operator to two different places. `engagement_outbox.status` still records it as `failed`: that ENUM +has no `bounced`, and from the queue's point of view a bounced row is one that finished +unsuccessfully. `complained` still has no writer, and cannot have one without a provider feedback loop. + +**Verified on the live rig**, seven rungs, each one a real send or a real refusal: + +1. baseline — three recipients, three mails in mailpit, three `sent` rows; +2. one address suppressed by hand — **two** mails, and a `suppressed` row naming the reason; +3. suppression lifted — three mails again; +4. gate `on` — two mails, `ineligible: { unverified: 1 }`, and **no send-log row at all** for the + excluded user, which is what enqueue-time exclusion means; +5. a `550 5.1.1` from the relay — two mails, one `bounced` row, and `b***@example.test` written to the + suppression list with `detail: hard bounce: 5.1.1`; +6. the same rule again — that address `suppressed`, with no transport call; +7. **a password reset to the suppressed address still arrived**, which is decision 2 proved rather + than asserted. + +The three admin routes were then driven over real HTTP: `created_by` records the admin on a manual row +and stays **NULL** on the worker's automatic one, which is what separates them in the list; a repeat +POST answers `200 {created:false}` rather than a 409; DELETE matches case-insensitively, which is the +whole point of hashing a folded address; no response contains `address_hash`; and all three refuse a +signed-out caller. + +**Still later phases':** an API-based transport with a signed webhook receiver, which is what would +bring asynchronous bounces and `complained` to life. Nothing here blocks it — a transport may declare +a bounce handler today. + --- ### Phase 10 — Protocol bump: `house.decay` enrichment *(parallel from day one)* @@ -3073,6 +3188,13 @@ 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. + + **The narrower half is settled (2026-08-31, at the start of Phase 9): an unverified address is + excluded at ENQUEUE, and only from the email channel.** With the gate on, `emailChannel.eligible` + drops the user before an outbox row is written, so nothing is queued only to be thrown away and + the admin reach preview can report the exclusion. It is email-only because a rule spanning + channels must still put an item in that person's in-app inbox. Transactional mail — resets, + invites, verification itself — is unaffected either way. 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