feat(engagement): deliverability — suppression, bounces and the verification gate
All checks were successful
PR Checks / client-build (pull_request) Successful in 36s
PR Checks / bot-tests (pull_request) Successful in 36s
PR Checks / server-tests (pull_request) Successful in 5m12s

ENGAGEMENT.md Phase 9, closing gap G16. Two mechanisms decide that somebody in a
rule's audience does not get the mail, and they sit at deliberately different
points in the pipeline.

`engagement_suppressions` is checked at DELIVERY: 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 with no transport call at all.

The Phase 1b verification gate is applied at ENQUEUE, through a new optional
`registerDeliveryChannel({ eligible })` that only `email` declares. Filtering the
shared audience would have silenced the wrong sink: a rule spanning email and
in-app must still put an item in an unverified user's inbox. The excluded counts
reach `summary.ineligible` and the admin reach preview, which until now reported
an audience size that was never the number of people who would be mailed.

`bounceClassify.js` is the only thing that may write a `bounce` row, and it is
deliberately NOT `mailer.PERMANENT_CODES`. That set answers "is retrying
pointless?" and contains EAUTH and 554 — an auth failure and a relay-wide policy
refusal, neither of which is a fact about the recipient. Reusing it would mean one
stale SMTP password suppressing every address the worker touched, silently. The
classifier reads the RFC 3463 enhanced status first, falls back to a phrase match
only past a veto list and only for 550/551/553, and does not suppress anything it
is unsure about.

Scope is engagement rules only: resets, invites, verification and the contact form
still attempt, matching the posture passwordReset.controller.js already stated.

Found on the live rig, against a real MariaDB and a real SMTP conversation: a hard
bounce was being recorded as `failed`, so the Send Log's "Bounced" filter — a
status `engagement_sends` has carried since §4.5 — matched nothing and always
would have. It is now its own outcome; the outbox row stays `failed`, since that
ENUM has no `bounced` and a bounced row is one that finished unsuccessfully.

`address_masked` is this phase's one addition to §4.5's DDL. A hash-only table
cannot be operated — an operator cannot tell three typos from a whole domain
refusing mail — and the domain survives while the local part is destroyed, so the
column can never be read back as an address book.

- schema: `engagement_suppressions` (+ `address_masked`, `created_by`)
- `GET/POST/DELETE /api/v1/admin/engagement/suppressions`, and Admin → Engagement
  → Suppressions, the only way out of the list
- `sendNotification` returns `smtp: { code, responseCode, response }`
- 26 new tests; swagger, routes manifest and guards regenerated

Docs: RunicGateway/docs#191.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-31 10:47:34 -05:00
parent 87c4e71025
commit c208543044
22 changed files with 2210 additions and 37 deletions

View File

@@ -29,11 +29,12 @@
// `teamName` (a display string the cooldown keys on) and the scope is `team:12`.
// A Team renamed between the mail and the click must not orphan the link in it.
const crypto = require('crypto')
const rulesDb = require('../model/engagement/engagementRules.db')
const recipients = require('../model/engagement/engagementRecipients.db')
const templates = require('./templates')
const projection = require('./projection')
const suppressions = require('./suppressions')
const settings = require('../model/settings/settings.model')
const unsubscribeToken = require('../utils/unsubscribeToken')
const log = require('../utils/logger')('engagement')
@@ -54,12 +55,10 @@ const DEFAULT_TEMPLATE = 'notify.event'
const baseUrl = () => templates.baseUrl()
// Lower-cased first: a bounce reported for "Darrow@example.com" has to match the
// row written for "darrow@example.com", and a hash of two spellings is two
// different rows. The local part is technically case-sensitive per RFC 5321 and
// no relay anybody deploys treats it that way.
const hashAddress = (address) =>
crypto.createHash('sha256').update(String(address).trim().toLowerCase()).digest('hex')
// Re-exported rather than defined here since Phase 9: the send log's hash and
// the suppression list's key have to be the same function or a bounce never finds
// the row it belongs to. `suppressions.js` owns it, next to the masking.
const hashAddress = suppressions.hashAddress
/**
* The two unsubscribe URLs for one recipient of one scope, or nulls.
@@ -94,6 +93,52 @@ function unsubscribeUrls(userId, scopeKey) {
/** Where this channel would send to, or null. */
const addressFor = (userId) => recipients.addressFor(userId)
/**
* Narrow an audience to the users this channel may write an outbox row for
* (Phase 9, decision 4).
*
* **One gate, and it is the Phase 1b verification setting.** With
* `email_verification_required` on, a user whose address is unverified is
* excluded here rather than refused at delivery, and the org lead settled it that
* way for two reasons. It is a STANDING property — unlike a suppression, which
* can appear inside a `delay_seconds` window and therefore has to be re-checked
* at send time — so the outbox row would be written only to be thrown away. And a
* deployment that upgraded before verifying anybody has an audience that is
* almost entirely unverified: excluding at delivery would write a `suppressed`
* row per person per rule firing, which is a send log nobody can read.
*
* The count comes back so the admin reach preview can say "1,204 excluded:
* unverified" instead of quietly promising a number the engine will not deliver.
*
* **It fails OPEN, and the try/catch is load-bearing rather than defensive
* habit.** `settings.isEmailVerificationRequired` swallows its own errors and
* answers `off`, but `unverifiedAmong` does not, and an uncaught throw here does
* not fail one recipient — `applyRule` awaits this before the per-user loop, so
* it would abandon the whole rule for every channel it names. A database having
* a bad minute would become a rule that silently sent nothing, with a clean send
* log and nothing in the outbox to retry. Same direction as the suppression
* check, for the same reason: the recoverable mistake is mail going out, not mail
* silently stopping.
*/
async function eligible(userIds) {
const list = userIds || []
if (!list.length) return { userIds: [], excluded: {} }
try {
if (!(await settings.isEmailVerificationRequired())) {
return { userIds: list.slice(), excluded: {} }
}
const unverified = await recipients.unverifiedAmong(list)
if (!unverified.size) return { userIds: list.slice(), excluded: {} }
return {
userIds: list.filter((id) => !unverified.has(Number(id))),
excluded: { unverified: unverified.size },
}
} catch (err) {
log.error('verification gate could not be evaluated; not excluding anyone', { message: err.message })
return { userIds: list.slice(), excluded: {} }
}
}
/**
* Deliver one claimed outbox row.
*
@@ -130,11 +175,60 @@ async function deliver(row) {
log.debug('template variables had no value', { key, missing: rendered.missing })
}
// **The suppression check is HERE and not at enqueue** (Phase 9). An outbox
// row can sit through a `delay_seconds` grace window, and an address can hard
// bounce inside it — so the only check that can be correct is the one taken
// immediately before the transport call. It is also the check the acceptance
// criterion describes: a `suppressed` row in the send log, and no transport
// call at all.
const blocked = await suppressions.isSuppressed(to.address)
if (blocked) {
return {
ok: false,
suppressed: true,
detail: `address is suppressed (${blocked.reason})`,
addressHash: hashAddress(to.address),
}
}
const result = await mailer().sendNotification({ to: to.address, rendered, ...unsub })
// A failed send is where a hard bounce enters the system on SMTP, and the
// reason it is worth catching rather than waiting for an API provider: a
// single-recipient send refused at RCPT TO is a synchronous 5.1.1, which is
// the most valuable deliverability signal there is and it was already being
// thrown away. `considerFailure` is narrow — see bounceClassify.js — and its
// note goes into the log on BOTH outcomes, so "this failed and was not
// suppressed" says why.
if (result && !result.ok && result.smtp) {
const verdict = await suppressions.considerFailure({ address: to.address, error: result.smtp })
// A bounce is terminal by definition. Overriding `retry` matters because
// `PERMANENT_CODES` does not contain every code that can carry a 5.1.x, so
// without this a genuine dead mailbox could still be retried four more
// times — each one another refusal on our record with the relay.
if (verdict.suppressed) {
return {
ok: false,
retry: false,
// `engagement_sends.status` has carried 'bounced' since §4.5 and
// nothing wrote it until here, so the Send Log's "Bounced" filter
// matched nothing — the live rig is what showed that. It is a distinct
// status rather than a flavour of 'failed' because the two need
// different actions: a failure means look at the relay, and a bounce
// means that person's address is gone.
bounced: true,
transport: result.transport,
detail: `${result.detail}${verdict.note}`,
addressHash: hashAddress(to.address),
}
}
return { ...result, detail: `${result.detail}${verdict.note}`, addressHash: hashAddress(to.address) }
}
// The send log stores a sha256 of the address and never the address itself
// (schema.sql): enough to correlate a bounce in Phase 9, useless as a mailing
// list. Attached on every outcome, because a failure is exactly the row a
// bounce would need to be matched against.
// (schema.sql): enough to correlate a bounce, useless as a mailing list.
// Attached on every outcome, because a failure is exactly the row a bounce
// would need to be matched against.
return { ...result, addressHash: hashAddress(to.address) }
} catch (err) {
// See the header: a throw here would be retried as if it were the relay's
@@ -144,4 +238,4 @@ async function deliver(row) {
}
}
module.exports = { addressFor, deliver, unsubscribeUrls, hashAddress, DEFAULT_TEMPLATE }
module.exports = { addressFor, eligible, deliver, unsubscribeUrls, hashAddress, DEFAULT_TEMPLATE }