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>
424 lines
18 KiB
JavaScript
424 lines
18 KiB
JavaScript
// ── Deliverability: suppression and bounces (ENGAGEMENT.md Phase 9) ────────
|
|
//
|
|
// The phase's acceptance criteria, plus the two things building it showed were
|
|
// worth pinning because getting either wrong is silent:
|
|
//
|
|
// • **the classifier is not `PERMANENT_CODES`** — an EAUTH or a 5.7.1 policy
|
|
// refusal must not suppress anybody. Reusing that set would have meant one
|
|
// stale SMTP password suppressing every address the worker touched, and
|
|
// nothing would have said so.
|
|
// • **the verification gate excludes at ENQUEUE, on the email channel only** —
|
|
// a rule spanning email and in-app must still reach an unverified user's
|
|
// inbox, which is the failure a shared audience filter would have shipped.
|
|
//
|
|
// Point the DB at a closed port before requiring anything: the registries reach
|
|
// utils/discordAnnounce, which builds the pool at require time.
|
|
process.env.DB_HOST = '127.0.0.1'
|
|
process.env.DB_PORT = '59999'
|
|
|
|
const { test, beforeEach, afterEach, after } = require('node:test')
|
|
const assert = require('node:assert/strict')
|
|
|
|
const registries = require('../src/modules/registries')
|
|
const channels = require('../src/engagement/channels')
|
|
const engine = require('../src/engagement/engine')
|
|
const emailChannel = require('../src/engagement/emailChannel')
|
|
const bounceClassify = require('../src/engagement/bounceClassify')
|
|
const suppressions = require('../src/engagement/suppressions')
|
|
const templates = require('../src/engagement/templates')
|
|
const audiences = require('../src/engagement/audiences')
|
|
const worker = require('../src/utils/engagementWorker')
|
|
const mailer = require('../src/utils/mailer')
|
|
const rulesDb = require('../src/model/engagement/engagementRules.db')
|
|
const recipients = require('../src/model/engagement/engagementRecipients.db')
|
|
const suppressionsDb = require('../src/model/engagement/engagementSuppressions.db')
|
|
const settings = require('../src/model/settings/settings.model')
|
|
const db = require('../src/utils/db')
|
|
|
|
require('../src/engagement')
|
|
registries.registerCore()
|
|
|
|
after(() => db.close())
|
|
|
|
const saved = new Map()
|
|
function patch(mod, name, fn) {
|
|
if (!saved.has(mod)) saved.set(mod, new Map())
|
|
if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name])
|
|
mod[name] = fn
|
|
}
|
|
function restore() {
|
|
for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn
|
|
saved.clear()
|
|
}
|
|
|
|
const TRIGGER = 'team.forum.post'
|
|
|
|
let world
|
|
|
|
beforeEach(() => {
|
|
world = {
|
|
mails: [],
|
|
// address_hash → row
|
|
suppressed: new Map(),
|
|
verificationRequired: false,
|
|
unverified: new Set(),
|
|
sendResult: { ok: true, transport: 'smtp' },
|
|
}
|
|
|
|
patch(mailer, 'sendNotification', async (msg) => {
|
|
world.mails.push(msg)
|
|
return world.sendResult
|
|
})
|
|
patch(templates, 'renderByKey', async (key) => ({
|
|
subject: `[${key}]`,
|
|
html: '<p>x</p>',
|
|
text: 'x',
|
|
missing: [],
|
|
values: {},
|
|
}))
|
|
patch(recipients, 'addressFor', async (userId) => ({ address: `u${userId}@example.test` }))
|
|
patch(recipients, 'filterActive', async (ids) => ids)
|
|
patch(recipients, 'storedModes', async () => new Map())
|
|
patch(recipients, 'unverifiedAmong', async (ids) =>
|
|
new Set(ids.map(Number).filter((id) => world.unverified.has(id))))
|
|
patch(recipients, 'addressesFor', async (ids) =>
|
|
new Map(ids.map((id) => [Number(id), `u${id}@example.test`])))
|
|
patch(rulesDb, 'getById', async () => ({ id: 1, trigger_id: TRIGGER, template_keys: {} }))
|
|
patch(settings, 'isEmailVerificationRequired', async () => world.verificationRequired)
|
|
|
|
patch(suppressionsDb, 'get', async (hash) => world.suppressed.get(hash) || null)
|
|
patch(suppressionsDb, 'add', async (entry) => {
|
|
if (world.suppressed.has(entry.address_hash)) return false
|
|
world.suppressed.set(entry.address_hash, entry)
|
|
return true
|
|
})
|
|
patch(suppressionsDb, 'remove', async (hash) => world.suppressed.delete(hash))
|
|
patch(suppressionsDb, 'suppressedAmong', async (hashes) =>
|
|
new Set(hashes.filter((h) => world.suppressed.has(h))))
|
|
})
|
|
afterEach(restore)
|
|
|
|
const outboxRow = (over = {}) => ({
|
|
id: 1,
|
|
rule_id: 1,
|
|
trigger_id: TRIGGER,
|
|
user_id: 11,
|
|
channel: 'email',
|
|
subject_key: 'The Silver Hand',
|
|
scope_key: 'team:1',
|
|
payload: {},
|
|
attempts: 0,
|
|
...over,
|
|
})
|
|
|
|
const suppress = (address, reason = 'bounce') =>
|
|
suppressions.suppress({ address, reason, detail: 'seeded by the test' })
|
|
|
|
// ── The classifier: what counts as the recipient's fault ───────────────────
|
|
//
|
|
// The single most important group in this file. Everything below it assumes
|
|
// `classify` is right about which failures are about a person.
|
|
|
|
test('a 5.1.1 is a hard bounce', () => {
|
|
const v = bounceClassify.classify({
|
|
responseCode: 550,
|
|
response: '550 5.1.1 <a@b.test>: Recipient address rejected: User unknown',
|
|
})
|
|
assert.equal(v.suppress, true)
|
|
assert.equal(v.reason, 'bounce')
|
|
})
|
|
|
|
// The bug this whole file exists to prevent. `mailer.PERMANENT_CODES` contains
|
|
// EAUTH, so "terminal failure → suppress" would have emptied the mailing list
|
|
// the first time an operator's SMTP password expired — with a clean send log and
|
|
// no warning anywhere.
|
|
test('an authentication failure suppresses nobody', () => {
|
|
const v = bounceClassify.classify({ code: 'EAUTH', message: 'Invalid login: 535 5.7.8' })
|
|
assert.equal(v.suppress, false)
|
|
assert.match(v.reason, /not a recipient failure/)
|
|
})
|
|
|
|
test('a policy refusal suppresses nobody — 5.7.x is about us, not the address', () => {
|
|
const v = bounceClassify.classify({ responseCode: 554, response: '554 5.7.1 Message rejected by policy' })
|
|
assert.equal(v.suppress, false)
|
|
})
|
|
|
|
test('a full mailbox is not a dead one, with or without an enhanced code', () => {
|
|
assert.equal(bounceClassify.classify({ responseCode: 552, response: '552 5.2.2 Mailbox full' }).suppress, false)
|
|
// The veto list earning its place: "mailbox unavailable" is in the
|
|
// no-such-mailbox phrases, and this sentence contains it.
|
|
assert.equal(
|
|
bounceClassify.classify({ responseCode: 550, response: '550 Mailbox unavailable: mailbox is full' }).suppress,
|
|
false,
|
|
)
|
|
})
|
|
|
|
test('a temporary failure suppresses nobody', () => {
|
|
assert.equal(
|
|
bounceClassify.classify({ responseCode: 451, response: '451 4.7.1 Greylisted, try again later' }).suppress,
|
|
false,
|
|
)
|
|
})
|
|
|
|
test('a bare 550 with an unambiguous reason still bounces', () => {
|
|
assert.equal(bounceClassify.classify({ responseCode: 550, response: '550 No such user here' }).suppress, true)
|
|
})
|
|
|
|
test('a bare 554 does not, because "transaction failed" means nothing in particular', () => {
|
|
assert.equal(bounceClassify.classify({ responseCode: 554, response: '554 Transaction failed' }).suppress, false)
|
|
})
|
|
|
|
// A refusal this file has no opinion about must be a refusal, not a guess. The
|
|
// asymmetry is deliberate: mailing a dead address again costs a retry, and
|
|
// suppressing a live one costs a person who silently stops hearing from us.
|
|
test('an unrecognised permanent code is not suppressed', () => {
|
|
const v = bounceClassify.classify({ responseCode: 550, response: '550 5.4.1 Access denied' })
|
|
assert.equal(v.suppress, false)
|
|
assert.match(v.reason, /not a known recipient failure/)
|
|
})
|
|
|
|
// A bounce that quotes another relay's answer carries two enhanced codes. The
|
|
// one that matters is the one THIS relay gave us, which is the one at the front.
|
|
test('the enhanced code is read from the front of the response, not found anywhere in it', () => {
|
|
const v = bounceClassify.classify({
|
|
responseCode: 554,
|
|
response: '554 5.7.1 rejected; remote host said: 550 5.1.1 user unknown',
|
|
})
|
|
assert.equal(v.suppress, false)
|
|
})
|
|
|
|
// ── The address key ────────────────────────────────────────────────────────
|
|
|
|
test('the hash is case- and whitespace-folded, so a bounce finds the row it belongs to', () => {
|
|
assert.equal(suppressions.hashAddress(' Darrow@Example.TEST '), suppressions.hashAddress('darrow@example.test'))
|
|
})
|
|
|
|
test('the mask keeps the domain and destroys the local part', () => {
|
|
assert.equal(suppressions.maskAddress('darrow@example.test'), 'd***@example.test')
|
|
// Two characters is most of a two-character local part, so nothing is kept.
|
|
assert.equal(suppressions.maskAddress('ab@example.test'), '***@example.test')
|
|
assert.equal(suppressions.maskAddress('not-an-address'), null)
|
|
})
|
|
|
|
// ── The acceptance criteria ────────────────────────────────────────────────
|
|
|
|
test('a suppressed address is skipped with no transport call at all', async () => {
|
|
await suppress('u11@example.test')
|
|
const result = await emailChannel.deliver(outboxRow())
|
|
assert.equal(result.ok, false)
|
|
assert.equal(result.suppressed, true)
|
|
assert.equal(world.mails.length, 0, 'the transport must not be called')
|
|
assert.match(result.detail, /suppressed \(bounce\)/)
|
|
})
|
|
|
|
test("the worker writes that as status='suppressed' in both tables, not as a failure", async () => {
|
|
const finished = []
|
|
const logged = []
|
|
const outboxDb = require('../src/model/engagement/engagementOutbox.db')
|
|
const sendsDb = require('../src/model/engagement/engagementSends.db')
|
|
patch(outboxDb, 'claim', async () => true)
|
|
patch(outboxDb, 'finish', async (id, status, detail) => finished.push({ id, status, detail }))
|
|
patch(sendsDb, 'record', async (entry) => logged.push(entry))
|
|
|
|
await suppress('u11@example.test')
|
|
const status = await worker.processRow(outboxRow())
|
|
|
|
assert.equal(status, 'suppressed')
|
|
assert.equal(finished[0].status, 'suppressed')
|
|
assert.equal(logged[0].status, 'suppressed')
|
|
// The correlation key rides along even here: it is what a later manual audit
|
|
// matches a relay's own bounce report against.
|
|
assert.match(logged[0].address_hash, /^[0-9a-f]{64}$/)
|
|
})
|
|
|
|
test('a hard bounce suppresses the address, and the next send never reaches the relay', async () => {
|
|
world.sendResult = {
|
|
ok: false,
|
|
retry: false,
|
|
transport: 'smtp',
|
|
detail: 'the relay refused the message',
|
|
smtp: { code: 'EENVELOPE', responseCode: 550, response: '550 5.1.1 User unknown' },
|
|
}
|
|
const first = await emailChannel.deliver(outboxRow())
|
|
assert.equal(first.ok, false)
|
|
assert.equal(world.mails.length, 1, 'the first attempt does reach the transport')
|
|
assert.match(first.detail, /hard bounce: 5\.1\.1/)
|
|
|
|
world.sendResult = { ok: true, transport: 'smtp' }
|
|
const second = await emailChannel.deliver(outboxRow())
|
|
assert.equal(second.suppressed, true)
|
|
assert.equal(world.mails.length, 1, 'the second attempt does not')
|
|
})
|
|
|
|
// Without this a genuine dead mailbox could be retried four more times, because
|
|
// PERMANENT_CODES does not contain every reply code that can carry a 5.1.x —
|
|
// each retry another refusal on our record with the relay.
|
|
// `engagement_sends.status` has carried 'bounced' since §4.5 and nothing wrote
|
|
// it until this phase, so the Send Log's "Bounced" filter matched nothing. The
|
|
// live rig is what showed that; this is what keeps it fixed. The outbox row is
|
|
// still 'failed' — that ENUM has no 'bounced', and from the queue's point of
|
|
// view a bounced row is simply one that finished unsuccessfully.
|
|
test("a hard bounce is logged as 'bounced', while the outbox row is 'failed'", async () => {
|
|
const finished = []
|
|
const logged = []
|
|
const outboxDb = require('../src/model/engagement/engagementOutbox.db')
|
|
const sendsDb = require('../src/model/engagement/engagementSends.db')
|
|
patch(outboxDb, 'claim', async () => true)
|
|
patch(outboxDb, 'finish', async (id, status) => finished.push(status))
|
|
patch(sendsDb, 'record', async (entry) => logged.push(entry))
|
|
|
|
world.sendResult = {
|
|
ok: false,
|
|
retry: true,
|
|
transport: 'smtp',
|
|
detail: 'refused',
|
|
smtp: { code: 'EENVELOPE', responseCode: 550, response: '550 5.1.1 User unknown' },
|
|
}
|
|
const status = await worker.processRow(outboxRow())
|
|
|
|
assert.equal(status, 'bounced')
|
|
assert.equal(logged[0].status, 'bounced')
|
|
assert.equal(finished[0], 'failed')
|
|
})
|
|
|
|
test('a bounce is terminal even when the mailer called the failure retryable', async () => {
|
|
world.sendResult = {
|
|
ok: false,
|
|
retry: true,
|
|
transport: 'smtp',
|
|
detail: 'refused',
|
|
smtp: { code: null, responseCode: 550, response: '550 5.1.1 User unknown' },
|
|
}
|
|
const result = await emailChannel.deliver(outboxRow())
|
|
assert.equal(result.retry, false)
|
|
})
|
|
|
|
test('a failure that is NOT a bounce stays retryable and suppresses nobody', async () => {
|
|
world.sendResult = {
|
|
ok: false,
|
|
retry: true,
|
|
transport: 'smtp',
|
|
detail: 'connection refused',
|
|
smtp: { code: 'ECONNECTION', responseCode: null, response: null },
|
|
}
|
|
const result = await emailChannel.deliver(outboxRow())
|
|
assert.equal(result.retry, true)
|
|
assert.equal(world.suppressed.size, 0)
|
|
// The send log says why it was not suppressed, so the non-event is explained
|
|
// rather than merely absent.
|
|
assert.match(result.detail, /not suppressed/)
|
|
})
|
|
|
|
// The suppression list must never be able to stop mail going out. A database
|
|
// that cannot answer "is this suppressed" is a database problem, and turning it
|
|
// into total silence with a clean send log is G22's shape all over again.
|
|
test('a suppression check that throws sends the mail anyway', async () => {
|
|
patch(suppressionsDb, 'get', async () => { throw new Error('db is down') })
|
|
const result = await emailChannel.deliver(outboxRow())
|
|
assert.equal(result.ok, true)
|
|
assert.equal(world.mails.length, 1)
|
|
})
|
|
|
|
test('the first reason an address was suppressed is the one that survives', async () => {
|
|
await suppress('u11@example.test', 'bounce')
|
|
await suppressions.suppress({ address: 'u11@example.test', reason: 'manual' })
|
|
const row = world.suppressed.get(suppressions.hashAddress('u11@example.test'))
|
|
assert.equal(row.reason, 'bounce')
|
|
})
|
|
|
|
test('un-suppressing is the way back, and it is the only one', async () => {
|
|
await suppress('u11@example.test')
|
|
assert.equal(await suppressions.unsuppress('u11@example.test'), true)
|
|
const result = await emailChannel.deliver(outboxRow())
|
|
assert.equal(result.ok, true)
|
|
})
|
|
|
|
// ── The verification gate (§7.1 Q1's narrower half) ────────────────────────
|
|
|
|
test('with the gate off, an unverified address is mailed', async () => {
|
|
world.unverified.add(11)
|
|
const gated = await channels.eligibleFor('email', [11, 12])
|
|
assert.deepEqual(gated.userIds, [11, 12])
|
|
assert.deepEqual(gated.excluded, {})
|
|
})
|
|
|
|
test('with the gate on, an unverified user is excluded and counted', async () => {
|
|
world.verificationRequired = true
|
|
world.unverified.add(11)
|
|
const gated = await channels.eligibleFor('email', [11, 12])
|
|
assert.deepEqual(gated.userIds, [12])
|
|
assert.deepEqual(gated.excluded, { unverified: 1 })
|
|
})
|
|
|
|
// The reason the gate is a CHANNEL hook rather than a filter on the shared
|
|
// audience. A rule spanning both channels must still put an item in an
|
|
// unverified user's inbox — being unverified is a reason not to mail somebody
|
|
// and no reason at all to hide their notifications from them.
|
|
test('the gate is email-only: in-app still reaches an unverified user', async () => {
|
|
world.verificationRequired = true
|
|
world.unverified.add(11)
|
|
const inapp = await channels.eligibleFor('inapp', [11, 12])
|
|
assert.deepEqual(inapp.userIds, [11, 12])
|
|
const push = await channels.eligibleFor('push', [11, 12])
|
|
assert.deepEqual(push.userIds, [11, 12])
|
|
})
|
|
|
|
// The gate must not be able to stop mail by failing, and the blast radius is
|
|
// wider than one recipient: `applyRule` awaits this before the per-user loop, so
|
|
// a throw would abandon the whole rule for every channel it names — a rule that
|
|
// silently sent nothing, with a clean send log and an empty outbox.
|
|
test('a gate that cannot be read is off, for every recipient and every channel', async () => {
|
|
patch(settings, 'isEmailVerificationRequired', async () => { throw new Error('db is down') })
|
|
const gated = await channels.eligibleFor('email', [11, 12])
|
|
assert.deepEqual(gated.userIds, [11, 12])
|
|
assert.deepEqual(gated.excluded, {})
|
|
})
|
|
|
|
test('and so is a gate whose user lookup fails', async () => {
|
|
world.verificationRequired = true
|
|
patch(recipients, 'unverifiedAmong', async () => { throw new Error('db is down') })
|
|
const gated = await channels.eligibleFor('email', [11, 12])
|
|
assert.deepEqual(gated.userIds, [11, 12])
|
|
})
|
|
|
|
// ── The engine end to end ──────────────────────────────────────────────────
|
|
|
|
test('the engine writes no outbox row for an excluded user, and counts the exclusion', async () => {
|
|
const enqueued = []
|
|
const outboxDb = require('../src/model/engagement/engagementOutbox.db')
|
|
const cooldownsDb = require('../src/model/engagement/engagementCooldowns.db')
|
|
const sendsDb = require('../src/model/engagement/engagementSends.db')
|
|
patch(outboxDb, 'enqueue', async (row) => { enqueued.push(row); return enqueued.length })
|
|
patch(cooldownsDb, 'claim', async () => true)
|
|
patch(sendsDb, 'countSentSince', async () => 0)
|
|
patch(audiences, 'resolveForRule', async () => ({
|
|
userIds: [11, 12],
|
|
ceiling: 'authenticated',
|
|
dormant: false,
|
|
}))
|
|
patch(audiences, 'permitted', () => true)
|
|
// Both channels default to a mode that enqueues only where the user opted in;
|
|
// email is opt-in, so say so for both users.
|
|
patch(recipients, 'storedModes', async (ids) => new Map(ids.map((id) => [Number(id), 'instant'])))
|
|
|
|
world.verificationRequired = true
|
|
world.unverified.add(11)
|
|
|
|
const rule = {
|
|
id: 1,
|
|
trigger_id: TRIGGER,
|
|
channels: ['email', 'inapp'],
|
|
max_sends_per_hour: 100,
|
|
cooldown_seconds: 0,
|
|
delay_seconds: 0,
|
|
conditions: null,
|
|
}
|
|
const summary = await engine.applyRule(rule, { triggerId: TRIGGER, data: {}, subject: 's' }, new Date())
|
|
|
|
assert.deepEqual(summary.ineligible, { unverified: 1 })
|
|
const emailRows = enqueued.filter((r) => r.channel === 'email').map((r) => r.user_id)
|
|
const inappRows = enqueued.filter((r) => r.channel === 'inapp').map((r) => r.user_id)
|
|
assert.deepEqual(emailRows, [12], 'the unverified user gets no mail')
|
|
assert.deepEqual(inappRows.sort(), [11, 12], 'and still gets an inbox item')
|
|
})
|