diff --git a/client/src/routes/admin/views/EngagementRules.jsx b/client/src/routes/admin/views/EngagementRules.jsx index 1382a2e..482f578 100644 --- a/client/src/routes/admin/views/EngagementRules.jsx +++ b/client/src/routes/admin/views/EngagementRules.jsx @@ -482,6 +482,30 @@ function RuleEditor({ catalog, segments, rule, onSaved, onCancel }) { // ── The screen ───────────────────────────────────────────────────────────── +// ── The Phase 6 migration notice ─────────────────────────────────────────── +// +// Team notifications used to be sent with no operator configuration at all; +// ENGAGEMENT.md Phase 6 moved them onto rules, and the org lead's decision was to +// seed those rules DISABLED rather than carve an exception into "nothing is on by +// default". The consequence is a deployment whose Team email has stopped and +// nobody has been told — which is G22's failure mode with a different cause — so +// the screen that can fix it says so. +// +// It reads the RULES rather than a flag, so it disappears the moment one is +// switched on and comes back if every one is switched off again. A deployment +// that deleted them all sees nothing, which is right: they made that choice. +const TEAM_TRIGGERS = [ + 'team.forum.post', + 'team.announcement', + 'team.member.joined', + 'team.leadership.changed', +] + +function teamRulesAllOff(rules) { + const team = rules.filter((r) => TEAM_TRIGGERS.includes(r.triggerId || r.trigger_id)) + return team.length > 0 && team.every((r) => !r.enabled) +} + export default function EngagementRules() { const [catalog, setCatalog] = useState(null) const [segments, setSegments] = useState([]) @@ -558,6 +582,21 @@ export default function EngagementRules() { return (
+ {teamRulesAllOff(rules) && ( +
+ Team notification emails are off. They used to be sent automatically; they + are now rules, and the four below arrived switched off so that nothing starts mailing on its + own. Switch on the ones this deployment wants — per-member preferences and per-Team mutes + still apply above them, and unsubscribe links in mail already sent still work. +
+ )} +

A rule turns an event into mail: which event, who hears about it, on which channels, and how diff --git a/server/db/schema.sql b/server/db/schema.sql index 59ee52b..a56b5fc 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1891,3 +1891,64 @@ CREATE TABLE IF NOT EXISTS engagement_templates ( INDEX idx_engt_trigger (trigger_id, channel, status), INDEX idx_engt_seed (seed_key) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- ── The email channel on the engine (ENGAGEMENT.md §4.2b — Phase 6) ───────── + +-- The scope an event is ABOUT, opaque to core and distinct from `subject_key`. +-- +-- They are two different things and Phase 6 is where that stopped being +-- theoretical. `subject_key` is what a COOLDOWN is keyed on and comes from the +-- trigger's declared `subjectKey` — for the four Team triggers that is `teamName`, +-- a display string. `scope_key` is what a PREFERENCE and an UNSUBSCRIBE are keyed +-- on, and it has to be a stable identifier: `team:12` survives a rename, and a +-- Team renamed between the mail and the click must not orphan the unsubscribe +-- link in it. Same vocabulary as engagement_digest_state.scope_key below. +ALTER TABLE engagement_outbox ADD COLUMN IF NOT EXISTS scope_key VARCHAR(190) NULL; + +-- §4.2b: digest state, and DELIBERATELY not a digest queue. +-- +-- The generic engine enqueues an outbox row per (rule, user, channel) at emit +-- time, carrying a snapshot of the payload. That is right for an instant send and +-- wrong for a digest, and `teamDigestWorker`'s header says why in three +-- properties: a deployment down for two days sends ONE digest rather than two +-- days of replay; a post a moderator hid after it was written is not in the +-- query so it is not in the mail; and a user who lost forum access between the +-- post and the send is no longer in the recipient set. The third is a security +-- property, and all three are properties of RE-DERIVING the content at send time. +-- A snapshot taken at emit time has none of them. +-- +-- So a digest-mode recipient gets NO outbox row (see engine.js), and what +-- generalizes is this: the state the worker keeps, lifted out of +-- team_notification_prefs.last_digest_at so that a second digest — on another +-- channel, or over another scope — needs no second column on somebody's +-- preferences table. +CREATE TABLE IF NOT EXISTS engagement_digest_state ( + user_id INT NOT NULL, + channel VARCHAR(32) NOT NULL, + -- '' is deployment-wide; 'team:12' is one Team. NOT NULL with a '' default + -- because this is a PRIMARY KEY column and MariaDB coerces a nullable one + -- anyway — the same workaround team_integration_config and teams.active_key + -- both carry, and the trap Part 4's preamble flags. + scope_key VARCHAR(190) NOT NULL DEFAULT '', + last_digest_at DATETIME NULL, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, channel, scope_key), + CONSTRAINT fk_engd_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + -- The worker's driving question is "whose email digest is due?", which is a + -- range scan of this index rather than of every digest ever sent. + INDEX idx_engd_due (channel, last_digest_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- Carry the Team digest windows across, once. Replay-safe by construction: an +-- INSERT IGNORE against the primary key, so the second and every later boot +-- writes nothing, and a window the new worker has since MOVED is not dragged +-- backwards by the next restart. +-- +-- Rows with a NULL last_digest_at are copied too, and that is deliberate rather +-- than incidental: `clampSince` treats a missing row and a NULL stamp the same +-- way (reach back one interval, not to the floor), so the copy is faithful — and +-- copying only the stamped rows would make the backfill's own idempotence depend +-- on which rows happened to have fired. +INSERT IGNORE INTO engagement_digest_state (user_id, channel, scope_key, last_digest_at) + SELECT user_id, 'email', CONCAT('team:', team_id), last_digest_at + FROM team_notification_prefs; diff --git a/server/db/seed.js b/server/db/seed.js index 41c92ee..1b1a93b 100644 --- a/server/db/seed.js +++ b/server/db/seed.js @@ -5,6 +5,7 @@ const wikiDb = require('../src/model/wiki/wiki.db') const users = require('../src/model/users/users.model') const { ensureSchema, close } = require('../src/utils/db') const { seedTemplates } = require('../src/engagement/templates') +const { seedTeamRules } = require('../src/engagement/coreRules') const brand = require('../src/config/brand') const log = require('../src/utils/logger')('seed') @@ -81,6 +82,10 @@ async function seedDefaults() { // failed to seed costs the shipped default, which `renderByKey` falls back to // anyway, and must not stop a boot. await seedTemplates() + // The four Team rules, seeded ONCE and all disabled (ENGAGEMENT.md Phase 6). + // Guarded by a settings key rather than re-ensured, so a rule an operator + // deleted stays deleted and one they enabled stays enabled. + await seedTeamRules() log.info('settings and wiki defaults ensured') } diff --git a/server/routes.guards.json b/server/routes.guards.json index 608f045..0dc2568 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -2068,6 +2068,18 @@ "validate" ] }, + { + "method": "GET", + "path": "/api/v1/public/engagement/unsubscribe/:token", + "handlers": 1, + "gates": [] + }, + { + "method": "POST", + "path": "/api/v1/public/engagement/unsubscribe/:token", + "handlers": 1, + "gates": [] + }, { "method": "GET", "path": "/api/v1/public/modules", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index 614ed32..b4e6fab 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -837,6 +837,14 @@ "method": "POST", "path": "/api/v1/public/contact" }, + { + "method": "GET", + "path": "/api/v1/public/engagement/unsubscribe/:token" + }, + { + "method": "POST", + "path": "/api/v1/public/engagement/unsubscribe/:token" + }, { "method": "GET", "path": "/api/v1/public/modules" diff --git a/server/src/emailBlocks/types/button.js b/server/src/emailBlocks/types/button.js index 513fa12..704e7d1 100644 --- a/server/src/emailBlocks/types/button.js +++ b/server/src/emailBlocks/types/button.js @@ -52,7 +52,15 @@ registerEmailBlock({ // still renders, inert, because dropping it silently would hide from the // reader that the mail was meant to offer them something. if (ctx.t(props.url).trim() === '') return '' - const href = ctx.safeHref(props.url) + // ABSOLUTIZED, like `email.image` and `email.itemList` already do, and this + // was a real defect until Phase 6 put a rule-driven variable in here. A + // trigger's `url` variables are validated site-RELATIVE by construction + // (`engagementEmit.RELATIVE_URL`), so `{{actionUrl}}` interpolates to + // `/guilds/the-silver-anvil` and a mail client has no origin to resolve that + // against: the button rendered a dead link. `absolute()` returns null for a + // relative path when no base is configured, which falls into the inert-label + // branch below rather than shipping the broken href. + const href = ctx.absolute(ctx.safeHref(props.url)) const label = ctx.h(props.label) if (!href) { return ( @@ -78,8 +86,12 @@ registerEmailBlock({ ) }, toText(props, ctx) { - const url = ctx.t(props.url).trim() - if (url === '') return '' // see toHtml: no url, no block, in either part + const raw = ctx.t(props.url).trim() + if (raw === '') return '' // see toHtml: no url, no block, in either part + // The text part shows the same absolute URL the button links to. Falls back + // to the raw value rather than dropping the block: a reader who can see a + // relative path can still find the site, and `itemList` makes the same trade. + const url = ctx.absolute(raw) || raw const lead = props.textLead ? ctx.t(props.textLead).trim() : '' return lead ? `${lead}\n${url}` : url }, diff --git a/server/src/engagement/audiences.js b/server/src/engagement/audiences.js index 2d9d465..d7721f4 100644 --- a/server/src/engagement/audiences.js +++ b/server/src/engagement/audiences.js @@ -16,11 +16,11 @@ // The caller must not send. Falling back to the rule's plain `audience` // column would reach a different population than the one composed (§5.1a // rule 4), which is the failure mode this whole design exists to avoid. -// 3. `members` as a PLAIN audience resolves to nobody. It is the ceiling for -// "a module-declared list", and without a segment there is no list - core +// 3. `members` resolves to nobody unless something NAMED the list: a segment, or +// (from Phase 6) an event carrying its own access-checked recipient set. Core // knows no game vocabulary and cannot guess which members were meant. A rule -// saved that way is inert and visible as such, rather than quietly falling -// back to something wider. +// with neither is inert and visible as such, rather than quietly falling back +// to something wider. const registries = require('../modules/registries') const channels = require('./channels') @@ -104,13 +104,36 @@ async function resolveForRule(rule, event) { case 'authenticated': case 'everyone': return { userIds: await recipients.active(), ceiling: rule.audience, dormant: false, reason: null } - case 'members': + case 'members': { + // **The event may name its own list, and Phase 6 is why that exists.** + // `members` is the ceiling for "a module-declared list", and until this + // phase the only way to name one was a segment — an operator-composed tree + // over audiences with CONSTANT params. That cannot express "the members of + // the Team this particular post was in": the list is different for every + // firing, and nothing in a saved segment reads the event. + // + // So an emitter that has already computed an access-checked recipient set + // hands it over on the envelope, and this is where it is used. It is not a + // bypass of anything: the set is still filtered through `users.status` + // below, and the ceiling returned is still `members`, so the G24 re-check + // in the engine still refuses a rule whose trigger has since narrowed. + // What it removes is core having to guess a game's membership vocabulary — + // the thing this case's original comment said it could not do. + if (Array.isArray(event.recipientUserIds) && event.recipientUserIds.length) { + return { + userIds: await recipients.filterActive(event.recipientUserIds), + ceiling: 'members', + dormant: false, + reason: null, + } + } return { userIds: [], ceiling: 'members', dormant: false, - reason: 'a "members" audience needs a segment naming which list', + reason: 'a "members" audience needs a segment naming which list, or an event that carries one', } + } default: // Fails closed on an audience name the lattice does not know - the same // posture `ceilings.permits` takes, and for the same reason. diff --git a/server/src/engagement/channels.js b/server/src/engagement/channels.js index 804f844..0f551c3 100644 --- a/server/src/engagement/channels.js +++ b/server/src/engagement/channels.js @@ -44,6 +44,15 @@ const isMode = (value) => MODES.includes(value) * @param {string} def.defaultMode the mode that applies with no stored row * @param {boolean} def.supportsDigest may a preference for this channel be 'digest' * @param {string} [def.description] one line for the preferences screen + * @param {(userId: number) => Promise<{address: string}|null>} [def.addressFor] + * where this channel would send to, or null when it cannot reach the user + * @param {(row: object) => Promise<{ok?: boolean, retry?: boolean, transport?: string, detail?: string, addressHash?: string}>} + * [def.deliver] deliver one claimed outbox row. **Must not throw** — the + * worker treats a throw as a transient failure, which is the right guess + * and a worse answer than the channel's own classification. A channel + * without one is declared but not yet deliverable, which is exactly what + * `inapp` is until Phase 7; the worker finishes such a row `failed` and + * says so in the send log rather than pretending it was sent. */ function registerDeliveryChannel(def) { if (!def || typeof def !== 'object') throw new Error('registerDeliveryChannel: definition required') @@ -67,6 +76,16 @@ function registerDeliveryChannel(def) { if (defaultMode === 'digest' && !supportsDigest) { throw new Error(`registerDeliveryChannel(${id}): defaultMode 'digest' needs supportsDigest`) } + // Optional, but not optionally-typed. A channel registering `deliver: true` or + // a stale import that resolved to undefined would otherwise be a channel that + // silently never delivers — the failure Phase 3 deferred the whole behavioural + // half to avoid freezing, and the one the worker's "no delivery implementation + // yet" branch would report as if it were by design. + for (const fn of ['addressFor', 'deliver']) { + if (def[fn] !== undefined && typeof def[fn] !== 'function') { + throw new Error(`registerDeliveryChannel(${id}): ${fn} must be a function`) + } + } channels.set(id, { id, @@ -75,12 +94,22 @@ function registerDeliveryChannel(def) { carriesContent, defaultMode, supportsDigest, + addressFor: def.addressFor, + deliver: def.deliver, }) return id } -/** Every channel, in registration order. The preferences screen's column set. */ -const all = () => [...channels.values()].map((c) => ({ ...c })) +/** + * Every channel, in registration order. The preferences screen's column set. + * + * **Declarative fields only** — `addressFor` and `deliver` are stripped. This is + * what a route serializes, and a function on an object bound for `res.json` is a + * key that silently disappears rather than an error; keeping the boundary here + * means the API shape is decided in one place instead of by JSON.stringify. + */ +const all = () => + [...channels.values()].map(({ addressFor, deliver, ...declared }) => ({ ...declared })) /** Just the ids. */ const ids = () => [...channels.keys()] diff --git a/server/src/engagement/coreChannels.js b/server/src/engagement/coreChannels.js index 158c294..aa33dd6 100644 --- a/server/src/engagement/coreChannels.js +++ b/server/src/engagement/coreChannels.js @@ -16,6 +16,7 @@ // there is nowhere to express that generically". This is that place. const { registerDeliveryChannel } = require('./channels') +const emailChannel = require('./emailChannel') const CHANNELS = [ { @@ -53,6 +54,12 @@ const CHANNELS = [ // `team_notification_prefs.email_mode` already takes ('off' by default). defaultMode: 'off', supportsDigest: true, + // Phase 6: the first channel with a body. `supportsDigest` above is now load- + // bearing rather than aspirational — a 'digest' preference means the engine + // writes NO outbox row and the digest worker re-derives the content at send + // time (§4.2b), which is a different delivery path rather than a batched one. + addressFor: emailChannel.addressFor, + deliver: emailChannel.deliver, }, { id: 'inapp', diff --git a/server/src/engagement/coreRules.js b/server/src/engagement/coreRules.js new file mode 100644 index 0000000..54deade --- /dev/null +++ b/server/src/engagement/coreRules.js @@ -0,0 +1,149 @@ +// ── The four Team rules core ships, all of them OFF ──────────────────────── +// +// ENGAGEMENT.md Phase 6, decision 3. Before this phase the Team pipeline mailed +// people with no operator configuration at all: the code decided who was mailed +// and about what, and the only knobs were per-user. Phase 6 moves that decision +// onto rules — which default `enabled = 0`, and of which core seeds none. +// +// **So a straight migration would have stopped Team email on every existing +// deployment, silently.** The org lead's decision was to honour the invariant +// rather than carve an exception into it: the rules are seeded, and they are +// seeded OFF. Team email resumes when an operator opens Admin → Engagement → +// Rules and switches one on, and until then the admin screen says so in as many +// words (`EngagementRules.jsx`). The release note names it. +// +// The alternative — seeding them enabled so nothing changes for anybody — was +// considered and refused. "Nothing is seeded, nothing is on by default" is what +// makes a rules table safe to restore, import or replicate, and an exception +// carved for the one pipeline that predates the engine is an exception that has +// to be re-argued every time somebody reads the invariant. +// +// **Seeded once, not ensured on every boot**, and the difference matters: an +// operator who deletes a rule must not find it back after a restart. The guard is +// a settings key, the same mechanism a one-shot migration uses, so a deployment +// that has seen this seed never sees it again — deleted rules stay deleted, and +// an enabled rule stays enabled rather than being reset to off. + +const rulesDb = require('../model/engagement/engagementRules.db') +const settingsDb = require('../model/settings/settings.db') +const log = require('../utils/logger')('engagement') + +// The one-shot guard. Its VALUE is the timestamp, purely so an operator reading +// the settings table can tell when it ran; only its presence is read. +const SEEDED_KEY = 'engagement_team_rules_seeded' + +const RULES = [ + { + trigger_id: 'team.forum.post', + name: 'Team forum posts', + // `members`, which resolves to the recipient set the event carries — the + // access-checked list `teamNotify` has always computed. Not `authenticated`, + // and the trigger's own ceiling would refuse that anyway: a private Team's + // forum excerpt reaching the whole site is the failure G24 exists for. + audience: 'members', + channels: ['email'], + // `email` is the instant body; `digest` is what the digest worker renders. + // Two keys because they are two different messages — a template written for + // one post renders a day of them as a single missing variable. + template_keys: { email: 'notify.team-post', digest: 'notify.digest' }, + // No cooldown. A busy thread is exactly what the per-user `email_mode` and + // the digest option are for, and a cooldown here would silently drop the + // second reply of a conversation rather than batching it. + cooldown_seconds: 0, + max_sends_per_hour: 500, + }, + { + trigger_id: 'team.announcement', + name: 'Team announcements', + audience: 'members', + channels: ['email'], + // **The generic body, not `notify.team-post`, and the reason is a naming + // inconsistency in the Phase 2 declarations rather than a design choice + // here.** The two triggers describe the same underlying thing — a thread in a + // Team forum — but `team.forum.post` declares its title as `threadTitle` and + // `team.announcement` declares it as `title`. A template can only name one of + // them, so `notify.team-post`'s `{{threadTitle}}` renders empty for an + // announcement. `notify.event` + the structural projection gets it right + // (`title` is in the payload, `actionUrl` falls back to `postUrl`), and + // reconciling the two declarations is a version bump this phase did not take + // on its own authority. + template_keys: { email: 'notify.event', digest: 'notify.digest' }, + cooldown_seconds: 0, + max_sends_per_hour: 500, + }, + { + trigger_id: 'team.member.joined', + name: 'Team — new member', + audience: 'members', + channels: ['email'], + // The generic body: `notify.event` plus the structural projection renders it + // with no authoring (§4.6.1 property 1). A deployment that wants a better one + // duplicates the template and points this rule at the copy. + template_keys: { email: 'notify.event' }, + // An hour, per user per Team. This is the rule §6.4 argued should not exist + // as a sink at all — a fifteen-minute sweep, already on the activity feed — + // and the cooldown is what makes it survivable for the operator who wants it + // anyway: a guild recruiting ten people in an afternoon sends one mail. + cooldown_seconds: 3600, + max_sends_per_hour: 200, + }, + { + trigger_id: 'team.leadership.changed', + name: 'Team — leadership change', + audience: 'members', + channels: ['email'], + template_keys: { email: 'notify.event' }, + cooldown_seconds: 3600, + max_sends_per_hour: 200, + }, +] + +/** + * Seed the four rules, once. Returns a small summary for the boot log. + * + * Never throws: it is on the boot path beside `seedTemplates`, and a rule that + * failed to seed costs an operator one visit to the "new rule" form, not a + * deployment. + */ +async function seedTeamRules() { + const summary = { inserted: 0, skipped: 0 } + try { + const seen = await settingsDb.get(SEEDED_KEY) + if (seen) return { ...summary, skipped: RULES.length } + + for (const rule of RULES) { + try { + await rulesDb.insert({ + audience_segment_id: null, + conditions: null, + // No delay and nothing cancels these. `delay_seconds` is the grace + // window a cancelling event needs, and nothing cancels "someone + // posted" — the post happened. + delay_seconds: 0, + cancel_on: [], + ...rule, + enabled: 0, + updated_by: null, + }) + summary.inserted += 1 + } catch (err) { + log.error('team rule seed failed', { trigger: rule.trigger_id, message: err.message }) + } + } + // Stamped even on a partial run. Re-running would duplicate the rules that + // did insert, and a duplicate rule is two mails per event — a worse outcome + // than the one missing rule an operator can add from the screen. + await settingsDb.set(SEEDED_KEY, new Date().toISOString()) + if (summary.inserted) { + log.info('seeded Team engagement rules, all disabled', { + rules: summary.inserted, + note: 'Team email stays off until an operator enables one', + }) + } + } catch (err) { + log.error('team rule seeding failed', { message: err.message }) + } + return summary +} + +module.exports = { seedTeamRules, RULES, SEEDED_KEY } diff --git a/server/src/engagement/coreScopePrefs.js b/server/src/engagement/coreScopePrefs.js new file mode 100644 index 0000000..9b00470 --- /dev/null +++ b/server/src/engagement/coreScopePrefs.js @@ -0,0 +1,64 @@ +// ── Core's own scope-preference provider: Teams ──────────────────────────── +// +// ENGAGEMENT.md Phase 6, decision 4. `team_notification_prefs` stays exactly +// where it is and keeps exactly the meaning it has had since Teams shipped; this +// is the adapter that lets the generic engine read it without knowing what a Team +// is. Registered here rather than at the bottom of `scopedPrefs.js` for the same +// reason `coreChannels` and `transports/smtp` are: requiring a registry must not +// have the side effect of populating it. +// +// **The two columns say different things and the mapping is not symmetric.** +// +// - `muted` is the Team's master switch and it silences EVERY channel. That is +// what the toggle has always meant on the account screen ("mute this Team"), +// and narrowing it to email would be a behaviour change nobody asked for. Note +// this is belt-and-braces on the live path — `teamNotify.recipientIds` already +// excludes muted users before the event is emitted — and it is here anyway so +// the meaning survives an emitter that stops filtering. +// - `email_mode` says nothing about any other channel, so on push or in-app this +// provider returns no opinion and the stream-level preference decides. +// +// **Absence of a row means 'off' for email, and that is the whole reason this +// provider answers for every user rather than only for the rows it finds.** The +// column defaults to `'off'` and both recipient queries COALESCE to it: no row +// has always meant "this person has not asked for Team email". Deferring to the +// stream-level preference instead would mean a user who once switched on +// `team.forum.post` email in the channels screen starts receiving mail from every +// Team on the deployment — a widening, produced by a migration, of a preference +// they expressed about something else. + +const { registerScopePreference } = require('./scopedPrefs') +const teamNotify = require('../model/teams/teamNotify.model') + +// team_notification_prefs.email_mode → the three modes the engine speaks. The +// vocabularies differ by one word and only one word: 'immediate' predates +// `notification_channel_prefs`, whose ENUM says 'instant'. +const EMAIL_MODE = { off: 'off', immediate: 'instant', digest: 'digest' } + +async function modesFor(userIds, channel, scopeId) { + const teamId = Number(scopeId) + if (!Number.isInteger(teamId) || teamId < 1) return new Map() + + const rows = await teamNotify.prefsForTeam(userIds, teamId) + const byUser = new Map(rows.map((r) => [Number(r.user_id), r])) + const modes = new Map() + + for (const userId of userIds) { + const row = byUser.get(Number(userId)) + if (row && Number(row.muted)) { + modes.set(Number(userId), 'off') + continue + } + if (channel !== 'email') continue // no opinion; the stream preference decides + modes.set(Number(userId), EMAIL_MODE[(row && row.email_mode) || 'off'] || 'off') + } + return modes +} + +registerScopePreference({ + prefix: 'team', + label: 'Team', + modesFor, +}) + +module.exports = { modesFor, EMAIL_MODE } diff --git a/server/src/engagement/emailChannel.js b/server/src/engagement/emailChannel.js new file mode 100644 index 0000000..f801665 --- /dev/null +++ b/server/src/engagement/emailChannel.js @@ -0,0 +1,147 @@ +// ── The email DeliveryChannel: addressFor + deliver ──────────────────────── +// +// ENGAGEMENT.md Phase 6. Phase 3 declared this channel and deliberately left it +// behaviourless ("declaring a function nothing calls freezes a signature before +// anything has tried to use it"); this is the phase that has something to try it +// with, and the signature survived unchanged. +// +// **What it does is four lookups and one send**, and the order matters because +// each step is a way the mail should not go out: +// +// 1. the address — re-checked for `status = 'active'`, because a delayed +// row can outlive the account it was queued for +// 2. the rule — for its per-channel template key; the outbox row +// carries `rule_id` and FK CASCADE guarantees it exists +// 3. the values — the payload snapshot, plus §4.6.1's structural +// projection, plus this recipient's unsubscribe link +// 4. the template — `renderByKey`, which falls back to the shipped seed +// rather than failing, and refuses a draft +// 5. the send — `mailer.sendNotification`, which classifies rather +// than throwing +// +// **It never throws**, and that is a stronger statement than the worker's +// `try/catch` around it: a throw would be read as a transient failure and retried +// five times, so an unrenderable template would become five identical failures in +// the send log instead of one honest terminal row. +// +// **The unsubscribe link is per recipient and is built from `scope_key`, never +// from `subject_key`.** They differ for every Team event: the subject is +// `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 unsubscribeToken = require('../utils/unsubscribeToken') +const log = require('../utils/logger')('engagement') + +// **Required lazily, and it is a real cycle rather than a style preference.** +// `engagement/index.js` requires `coreChannels`, which requires this file; and +// `utils/mailer` requires `engagement/index` for the transport registry. A +// top-level `require('../utils/mailer')` here therefore resolves while +// `engagement/index` is mid-evaluation, so mailer would capture `{}` for +// `transports` and every send would fail on `transports.get is not a function` — +// at send time, on a deployment, with the boot log clean. Resolved at call time +// instead, by which point both modules are fully evaluated. +const mailer = () => require('../utils/mailer') + +// The template a rule renders through when it names none. §4.6.1 property 1: a +// new trigger must be mailable with no authoring at all, and this plus +// `projection.project` is that property's implementation. +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') + +/** + * The two unsubscribe URLs for one recipient of one scope, or nulls. + * + * TWO urls from one token, and they are not interchangeable. `unsubscribeUrl` is + * the human one that goes in the mail body: the site's own page, which explains + * what is about to happen and POSTs once a person has read it. `unsubscribeApiUrl` + * is the machine one for the `List-Unsubscribe` header, where RFC 8058 says a + * client may POST without showing anybody anything — so it has to be an endpoint, + * not a page. The API route answers GET on the same path with a redirect to the + * page, which covers clients that render the header as an ordinary link. + * + * A scope the token format cannot carry yields nulls rather than an exception: + * the mail is worth sending without a one-click unsubscribe, and the recipient + * still has the preferences screen. It is logged because it is a programming + * error in whatever chose the scope key. + */ +function unsubscribeUrls(userId, scopeKey) { + try { + const token = unsubscribeToken.sign(userId, 'email', scopeKey || '') + const base = baseUrl() + return { + unsubscribeUrl: `${base}/unsubscribe/${token}`, + unsubscribeApiUrl: `${base}/api/v1/public/engagement/unsubscribe/${token}`, + } + } catch (err) { + log.warn('could not build an unsubscribe link', { scope: scopeKey, message: err.message }) + return { unsubscribeUrl: null, unsubscribeApiUrl: null } + } +} + +/** Where this channel would send to, or null. */ +const addressFor = (userId) => recipients.addressFor(userId) + +/** + * Deliver one claimed outbox row. + * + * @returns {Promise<{ok: boolean, retry?: boolean, transport?: string, detail?: string}>} + */ +async function deliver(row) { + try { + const to = await addressFor(row.user_id) + if (!to) { + // Terminal. Retrying does not give somebody an address, and a banned + // account is not going to be un-banned by a five-minute backoff. + return { ok: false, detail: 'no deliverable address for this user' } + } + + const rule = await rulesDb.getById(row.rule_id) + const key = (rule && rule.template_keys && rule.template_keys.email) || DEFAULT_TEMPLATE + + // Once, not once per use: the body's link and the header's must be the same + // token, or a client that offers both offers two different unsubscribes. + const unsub = unsubscribeUrls(row.user_id, row.scope_key) + const values = projection.project(row.trigger_id, row.payload || {}, unsub) + + const rendered = await templates.renderByKey(key, values) + if (!rendered) { + // Neither a usable row nor a shipped seed. Terminal, and it names the key: + // the operator deleted a template a rule points at, which the admin surface + // refuses with a 409 — so reaching here means it happened out of band. + return { ok: false, detail: `no template and no shipped default for "${key}"` } + } + if (rendered.missing.length) { + // Not a refusal: an optional variable a trigger chose not to supply renders + // as nothing by design. Logged with NAMES ONLY, never values — the same + // rule the emit and dispatch log lines follow. + log.debug('template variables had no value', { key, missing: rendered.missing }) + } + + const result = await mailer().sendNotification({ to: to.address, rendered, ...unsub }) + // 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. + return { ...result, addressHash: hashAddress(to.address) } + } catch (err) { + // See the header: a throw here would be retried as if it were the relay's + // fault. Classified as terminal instead, with the reason in the send log. + log.error('email delivery failed', { outbox: row.id, message: err.message }) + return { ok: false, detail: `delivery error: ${err.message}` } + } +} + +module.exports = { addressFor, deliver, unsubscribeUrls, hashAddress, DEFAULT_TEMPLATE } diff --git a/server/src/engagement/engine.js b/server/src/engagement/engine.js index 82729d7..5157854 100644 --- a/server/src/engagement/engine.js +++ b/server/src/engagement/engine.js @@ -37,6 +37,7 @@ const recipients = require('../model/engagement/engagementRecipients.db') const conditions = require('./conditions') const audiences = require('./audiences') const channels = require('./channels') +const scopedPrefs = require('./scopedPrefs') const log = require('../utils/logger')('engagement') const HOUR_MS = 60 * 60 * 1000 @@ -52,23 +53,47 @@ const HOUR_MS = 60 * 60 * 1000 const liveChannels = (rule) => (rule.channels || []).filter((c) => channels.has(c)) /** - * Narrow a candidate set to the users whose EFFECTIVE mode for (id, channel) is - * not 'off'. + * The EFFECTIVE mode each candidate holds for (id, channel), given the event's + * scope. * * Effective, not stored: a row exists only where a user has expressed something, * and absence means the channel's `defaultMode` (§3.1). Reading the stored rows * and applying the default here keeps that answer in the registry, which is the * invariant Phase 3 established. * - * A 'digest' preference is kept, not dropped. Digest delivery is Phase 6's, and - * an outbox row for it is still the right record of "this person should be told"; - * what changes in Phase 6 is who drains it. + * **A scoped preference wins outright where one exists** (Phase 6, decision 4). + * `team_notification_prefs` stayed where it is and `scopedPrefs` is the adapter; + * for a Team-scoped event that table is the preference, exactly as it has been + * since Teams shipped. The argument for replacing rather than intersecting is in + * scopedPrefs.js's header, and it is short: intersecting would have silenced + * every existing Team-email subscriber on the deploy that migrated them. */ -async function subscribedTo(userIds, streamId, channel) { - if (!userIds.length) return [] +async function effectiveModes(userIds, streamId, channel, scopeKey) { + const modes = new Map() + if (!userIds.length) return modes + const scoped = await scopedPrefs.resolve(userIds, channel, scopeKey) const stored = await recipients.storedModes(userIds, streamId, channel) const fallback = channels.defaultMode(channel) - return userIds.filter((id) => (stored.get(id) ?? fallback) !== 'off') + for (const id of userIds) modes.set(id, scoped.get(id) ?? stored.get(id) ?? fallback) + return modes +} + +/** + * The candidates who should get an OUTBOX ROW for this channel. + * + * `off` is excluded for the obvious reason. **`digest` is excluded too, and that + * corrects what Phase 4a said here** — its comment read "a 'digest' preference is + * kept, not dropped… what changes in Phase 6 is who drains it", and what changed + * in Phase 6 is that nothing drains it. §4.2b keeps `teamDigestWorker`'s + * compute-at-send-time design, so a digest is re-derived from the source tables + * when it goes out, not assembled from snapshots taken hours earlier. An outbox + * row for a digest recipient would be a second copy of the content with none of + * the three properties that design exists for — most importantly, it would mail + * a user who lost access between the post and the send. + */ +async function subscribedTo(userIds, streamId, channel, scopeKey = null) { + const modes = await effectiveModes(userIds, streamId, channel, scopeKey) + return userIds.filter((id) => modes.get(id) === 'instant') } /** @@ -132,7 +157,7 @@ async function applyRule(rule, event, now) { const dueAt = new Date(now.getTime() + Math.max(0, rule.delay_seconds) * 1000) for (const channel of live) { - const eligible = await subscribedTo(resolved.userIds, event.triggerId, channel) + const eligible = await subscribedTo(resolved.userIds, event.triggerId, channel, event.scopeKey) for (const userId of eligible) { if (budget <= 0) { summary.capped += 1 @@ -151,6 +176,10 @@ async function applyRule(rule, event, now) { user_id: userId, channel, subject_key: subjectKey, + // The scope a PREFERENCE and an UNSUBSCRIBE are keyed on, which is not + // `subject_key`: for the Team triggers the subject is `teamName` (what a + // cooldown counts) and the scope is `team:12` (what survives a rename). + scope_key: event.scopeKey ?? null, payload: event.data, // Scoped per (rule, user, channel) by the unique index, so one event // fanned out to fifty people is fifty rows carrying the same key. @@ -238,4 +267,4 @@ async function dispatch(event, now = new Date()) { return summary } -module.exports = { dispatch, applyRule, applyCancellations, subscribedTo, liveChannels, HOUR_MS } +module.exports = { dispatch, applyRule, applyCancellations, subscribedTo, effectiveModes, liveChannels, HOUR_MS } diff --git a/server/src/engagement/index.js b/server/src/engagement/index.js index a815d78..d661461 100644 --- a/server/src/engagement/index.js +++ b/server/src/engagement/index.js @@ -20,6 +20,7 @@ require('./transports/smtp') require('./coreChannels') +require('./coreScopePrefs') const transports = require('./transports') const channels = require('./channels') diff --git a/server/src/engagement/projection.js b/server/src/engagement/projection.js new file mode 100644 index 0000000..df7aa13 --- /dev/null +++ b/server/src/engagement/projection.js @@ -0,0 +1,73 @@ +// ── The structural projection: any trigger through a generic template ────── +// +// ENGAGEMENT.md §4.6.1 property 1, implemented in Phase 6. The property is that +// **a new trigger renders through `notify.event` with no authoring at all** — +// "add a trigger" must not mean "and now write a template". Nothing implemented +// it before this phase, and building the email channel is what made the hole +// visible: a trigger payload is domain-named (`teamName`, `threadTitle`, +// `postUrl`) while the generic seeds are structural (`title`, `intro`, `items`, +// `actionUrl`). The two vocabularies never met. +// +// **The rule, settled by the org lead 2026-08-29: the payload wins, and the +// projection fills gaps.** A name the payload already carries is left exactly as +// emitted — `news.post` and `team.announcement` both declare their own `title`, +// and a projection that overwrote it would replace a real headline with a +// category label. Only a name the payload does NOT define is supplied here. +// +// **What it is careful not to do is guess at domain meaning.** There is no table +// mapping `threadTitle` onto `title`, and there will not be one: every such +// mapping is a piece of one game's vocabulary compiled into core, and it is wrong +// the first time a module names the same thing differently. The three fallbacks +// below are all derived from the DECLARATION — a trigger's own label, its own +// description, its own first declared url — which every trigger has by +// construction because `registerEventTriggers` refuses one without them. +// +// The consequence, stated plainly: an unauthored mail for `team.forum.post` is +// titled "Team — new forum post" rather than the thread's title. That is a plain +// mail, not a wrong one, and the operator's answer is the bespoke template that +// ships beside it (`notify.team-post` reads the payload's own names). A projection +// clever enough to do better would be a projection that is confidently wrong on +// the first module that does not follow core's naming. + +const registries = require('../modules/registries') + +/** + * The values a template renders with, for one event and one recipient. + * + * @param {string} triggerId + * @param {Record} payload the outbox row's snapshot — already + * validated at emit, so it holds declared variables and nothing else + * @param {Record} [extra] per-recipient additions the channel + * computes (`unsubscribeUrl`), merged LAST because they are facts about + * the delivery rather than about the event + * @returns {Record} + */ +function project(triggerId, payload = {}, extra = {}) { + const declaration = registries.eventTrigger(triggerId) + const values = { ...payload } + + // A dormant trigger still has an outbox row to deliver — the module was + // uninstalled between enqueue and now. The payload is intact and the template + // may well only reference payload names, so the mail goes out with whatever the + // snapshot holds rather than being refused for want of a label. + if (declaration) { + if (values.title === undefined) values.title = declaration.label + if (values.intro === undefined) values.intro = declaration.description || '' + if (values.actionUrl === undefined) { + const url = (declaration.variables || []).find( + (v) => v.type === 'url' && typeof payload[v.name] === 'string' && payload[v.name], + ) + if (url) values.actionUrl = payload[url.name] + } + } + + // Set rather than left absent, so a generic template's item list renders as + // nothing instead of reporting `items` as a missing variable. `missing` is what + // the editor's preview shows an operator, and a name no trigger was ever going + // to supply is noise in it. + if (values.items === undefined) values.items = [] + + return { ...values, ...extra } +} + +module.exports = { project } diff --git a/server/src/engagement/scopedPrefs.js b/server/src/engagement/scopedPrefs.js new file mode 100644 index 0000000..0784065 --- /dev/null +++ b/server/src/engagement/scopedPrefs.js @@ -0,0 +1,122 @@ +// ── Scoped preferences: "this channel, for this one Team" ────────────────── +// +// ENGAGEMENT.md Phase 6, decision 4. `notification_channel_prefs` is keyed +// (user, stream, channel) and has no scope column; `team_notification_prefs` is +// keyed (user, Team) and is the preference people actually hold today — someone +// in six Teams silences one. Migrating the second into the first would mean a +// live migration of user data, a wire-shape change on two clients, and the loss +// of the granularity in between. The org lead's decision was to keep the Team +// table and have the engine consult it; this file is the seam that lets it, +// without core's engine learning what a Team is. +// +// A registrant claims a scope PREFIX — the part of a scope key before the colon, +// `team` in `team:12` — and answers, for a set of users and one channel, what +// that scope says their mode is. +// +// **Where a scope answers, its answer REPLACES the stream-level preference; it +// does not intersect with it.** The decision was phrased as "a suppression below +// the channel preference", and building it showed that reading is the one that +// cannot ship: `notification_channel_prefs` holds a row only where a user has +// expressed something, absence means the channel's `defaultMode`, and email's is +// `off`. Nobody has ever expressed a stream-level opinion about `team.forum.post` +// — the screen that would let them is Phase 3's and the preference predates it — +// so intersecting would resolve every existing Team-email subscriber to `off` and +// silence the entire live pipeline on the deploy that migrated it. That is the +// G22 failure mode with a different cause. Replacement keeps today's behaviour +// byte-for-byte: for a Team-scoped event, `team_notification_prefs` is the +// preference, exactly as it has been since Teams shipped. +// +// The cost, stated so nobody has to rediscover it: a user cannot turn Team email +// off for every Team at once from the channels screen. That control lives on the +// per-Team screen, which is where it has always lived and where the unsubscribe +// link points. +// +// Nothing here caches. A preference read is one indexed query per (event, +// channel), against a table the user can change between two events. + +const log = require('../utils/logger')('engagement') + +// prefix → provider +const providers = new Map() + +const PREFIX_RE = /^[a-z][a-z0-9_-]*$/ + +/** + * Parse a scope key into its prefix and id. `''` and anything malformed are + * `null` — an unparseable scope must read as "no scope", never as some other + * scope's. + * + * @returns {{ prefix: string, id: string }|null} + */ +function parse(scopeKey) { + const raw = String(scopeKey || '') + const at = raw.indexOf(':') + if (at < 1 || at === raw.length - 1) return null + const prefix = raw.slice(0, at) + if (!PREFIX_RE.test(prefix)) return null + return { prefix, id: raw.slice(at + 1) } +} + +/** + * Register a scope-preference provider. + * + * Validate-then-commit, the same discipline the transport and channel registries + * use: every check runs before the map is touched. + * + * @param {object} def + * @param {string} def.prefix the scope-key prefix this provider owns, e.g. 'team' + * @param {string} def.label operator-facing, for the send log and admin copy + * @param {(userIds: number[], channel: string, scopeId: string) => Promise>} def.modesFor + * A mode per user for the users this scope has an opinion about. A user + * left OUT of the map defers to the stream-level preference; a user in it + * is answered by the scope. Must not throw — see `resolve`. + */ +function registerScopePreference(def) { + if (!def || typeof def !== 'object') throw new Error('registerScopePreference: definition required') + const { prefix, label, modesFor } = def + if (typeof prefix !== 'string' || !PREFIX_RE.test(prefix)) { + throw new Error(`registerScopePreference: invalid prefix ${JSON.stringify(prefix)}`) + } + if (providers.has(prefix)) throw new Error(`registerScopePreference: ${prefix} is already registered`) + if (typeof label !== 'string' || !label) throw new Error(`registerScopePreference(${prefix}): label required`) + if (typeof modesFor !== 'function') throw new Error(`registerScopePreference(${prefix}): modesFor required`) + providers.set(prefix, { prefix, label, modesFor }) + return prefix +} + +/** + * What does this scope say about these users on this channel? + * + * @returns {Promise>} empty when the scope is absent, + * unparseable, or owned by nobody — all three of which mean "this event + * is not scoped as far as preferences are concerned", which is the right + * answer for a module whose scope provider has been uninstalled. + */ +async function resolve(userIds, channel, scopeKey) { + const parsed = parse(scopeKey) + if (!parsed || !userIds.length) return new Map() + const provider = providers.get(parsed.prefix) + if (!provider) return new Map() + try { + const modes = await provider.modesFor(userIds, channel, parsed.id) + return modes instanceof Map ? modes : new Map() + } catch (err) { + // **Fails OPEN, and that is the uncomfortable choice made deliberately.** A + // provider that throws leaves the stream-level preference in charge, which + // for every core channel is `off` — so the practical effect of a failure is + // that nothing is sent, not that everybody is mailed. Failing closed by + // refusing the whole event would instead drop an IDOC warning because a Team + // preference query timed out. + log.error('scope preference lookup failed', { scope: scopeKey, channel, message: err.message }) + return new Map() + } +} + +const has = (prefix) => providers.has(prefix) + +// Test-only: the registry is module-level state. +function _reset() { + providers.clear() +} + +module.exports = { registerScopePreference, resolve, parse, has, _reset } diff --git a/server/src/engagement/templateSeeds.js b/server/src/engagement/templateSeeds.js index 9b6618e..745d641 100644 --- a/server/src/engagement/templateSeeds.js +++ b/server/src/engagement/templateSeeds.js @@ -236,21 +236,21 @@ const SEEDS = [ name: 'Team post notification', channel: 'email', protected: false, - seedVersion: 1, + seedVersion: 2, subject: '{{teamName}}: {{threadTitle}}', variables: [ { name: 'teamName', type: 'string', required: true, example: 'The Silver Anvil' }, { name: 'authorName', type: 'string', required: true, example: 'Aldric' }, { name: 'threadTitle', type: 'string', required: true, example: 'Meeting moved to Friday' }, { name: 'excerpt', type: 'string', required: false, example: 'We are pushing this week back a day so more people can make it.' }, - { name: 'threadUrl', type: 'string', required: false, example: 'https://example.com/teams/1?thread=9' }, + { name: 'postUrl', type: 'string', required: false, example: '/guilds/the-silver-anvil/forum/412' }, { name: 'unsubscribeUrl', type: 'string', required: false, example: 'https://example.com/unsubscribe/abc123' }, ], blocks: [ text('p1', '{{authorName}} posted in {{teamName}}.'), heading('h', '{{threadTitle}}', 'h2'), text('excerpt', '{{excerpt}}', { muted: true }), - button('cta', 'Read the thread', '{{threadUrl}}'), + button('cta', 'Read the thread', '{{postUrl}}'), divider('rule'), button('unsub', 'Unsubscribe', '{{unsubscribeUrl}}', 'To stop these emails for this team, use this link:'), ], diff --git a/server/src/model/engagement/engagementDigest.db.js b/server/src/model/engagement/engagementDigest.db.js new file mode 100644 index 0000000..424708d --- /dev/null +++ b/server/src/model/engagement/engagementDigest.db.js @@ -0,0 +1,62 @@ +// ── engagement_digest_state (ENGAGEMENT.md §4.2b, Phase 6) ───────────────── +// +// The state a digest keeps, and deliberately the ONLY state a digest keeps. What +// goes IN a digest is re-derived from the source tables when the mail is about to +// go out; this table answers one question — "what window does this person's next +// digest cover?" — and nothing else. +// +// Lifted out of `team_notification_prefs.last_digest_at`, where it was a worker's +// column sitting on a user's preferences row. Keyed (user, channel, scope) so a +// second digest — on another channel, or over another scope — needs no second +// column on somebody else's table. + +const { query } = require('../../utils/db') + +/** + * The stamps for a set of users in one scope, as a Map. + * + * Returns only the rows that exist. Absence is the CALLER's to interpret, and it + * matters that it is: `clampSince` treats a missing row and a NULL stamp + * identically (reach back one interval, not to the seven-day floor), so a person + * who has never had a digest and a person whose row was written by the backfill + * get the same first window. + */ +async function stampsFor(userIds, channel, scopeKey = '') { + const ids = [...new Set(userIds.map(Number).filter((n) => Number.isInteger(n) && n > 0))] + if (!ids.length) return new Map() + const rows = await query( + `SELECT user_id, last_digest_at FROM engagement_digest_state + WHERE channel = ? AND scope_key = ? AND user_id IN (${ids.map(() => '?').join(',')})`, + [channel, scopeKey, ...ids], + ) + return new Map(rows.map((r) => [Number(r.user_id), r.last_digest_at])) +} + +/** One user's stamp, or undefined. */ +async function stampFor(userId, channel, scopeKey = '') { + const rows = await query( + `SELECT last_digest_at FROM engagement_digest_state + WHERE user_id = ? AND channel = ? AND scope_key = ?`, + [Number(userId), channel, scopeKey], + ) + return rows.length ? rows[0].last_digest_at : undefined +} + +/** + * Stamp a digest as delivered. + * + * Written ONLY after a successful send, which is the property the old + * `stampDigest` had and the one worth restating: stamping first would silently + * eat a day of somebody's notifications every time the mail provider has a bad + * minute. + */ +async function stamp(userId, channel, scopeKey, at) { + await query( + `INSERT INTO engagement_digest_state (user_id, channel, scope_key, last_digest_at) + VALUES (?, ?, ?, ?) + ON DUPLICATE KEY UPDATE last_digest_at = VALUES(last_digest_at)`, + [Number(userId), channel, scopeKey || '', at], + ) +} + +module.exports = { stampsFor, stampFor, stamp } diff --git a/server/src/model/engagement/engagementOutbox.db.js b/server/src/model/engagement/engagementOutbox.db.js index 06aef38..54ddbe4 100644 --- a/server/src/model/engagement/engagementOutbox.db.js +++ b/server/src/model/engagement/engagementOutbox.db.js @@ -22,14 +22,18 @@ const hydrate = (row) => row && { ...row, payload: parseJson(row.payload, {}) } async function enqueue(row) { const result = await query( `INSERT IGNORE INTO engagement_outbox - (rule_id, trigger_id, user_id, channel, subject_key, payload, dedupe_key, due_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + (rule_id, trigger_id, user_id, channel, subject_key, scope_key, payload, dedupe_key, due_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, [ row.rule_id, row.trigger_id, row.user_id, row.channel, row.subject_key || '', + // NULL, not '', for an unscoped event: '' is a scope key that means + // "deployment-wide" in engagement_digest_state, and this column has to be + // able to say "no scope at all" as well. + row.scope_key ?? null, JSON.stringify(row.payload || {}), row.dedupe_key ?? null, row.due_at, diff --git a/server/src/model/engagement/engagementRecipients.db.js b/server/src/model/engagement/engagementRecipients.db.js index 3b85f80..f942615 100644 --- a/server/src/model/engagement/engagementRecipients.db.js +++ b/server/src/model/engagement/engagementRecipients.db.js @@ -122,4 +122,29 @@ const storedModes = async (userIds, streamId, channel) => { return new Map(rows.map((r) => [Number(r.user_id), r.mode])) } -module.exports = { active, staff, subscribers, filterActive, storedModes, MAX_AUDIENCE } +/** + * One user's mailable address, or null — the email channel's `addressFor` + * (Phase 6). + * + * `status = 'active'` is re-checked here even though every audience query already + * filtered on it, and the gap it closes is real rather than theoretical: an + * outbox row can sit through a `delay_seconds` grace window, so a user banned + * between the emit and the send is exactly the case this catches. The cost is one + * primary-key lookup on a path that is about to open an SMTP conversation. + * + * **It does not gate on `email_verified`.** Whether an unverified address may + * receive opt-in mail is §7.1 Q1's narrower half, and it is a Phase 9 decision + * with the suppression list in front of it; deciding it here by accident would + * mean every deployment that upgraded before verifying its users stopped mailing + * them. + */ +const addressFor = async (userId) => { + const rows = await query( + `SELECT email FROM users + WHERE id = ? AND status = 'active' AND email IS NOT NULL AND email <> ''`, + [Number(userId)], + ) + return rows.length ? { address: rows[0].email } : null +} + +module.exports = { active, staff, subscribers, filterActive, storedModes, addressFor, MAX_AUDIENCE } diff --git a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.db.js b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.db.js index 9176548..806de14 100644 --- a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.db.js +++ b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.db.js @@ -41,4 +41,27 @@ async function offPushExcept(userId, keep) { ) } -module.exports = { listByUser, upsert, offPushExcept } +/** + * Turn one channel off for every named stream — the deployment-wide unsubscribe + * (ENGAGEMENT.md Phase 6). + * + * It WRITES a row per stream rather than updating the rows that happen to exist, + * and the difference is the same one `offPushExcept` argues: absence means the + * channel's `defaultMode`, so updating only what is there would leave a user + * unsubscribed today and re-subscribed the day a channel ships a non-off default. + * An unsubscribe has to be a statement, not the absence of one. + */ +async function offForChannel(userId, channel, streamIds) { + const ids = [...new Set(streamIds)].filter((s) => typeof s === 'string' && s) + if (!ids.length) return null + const values = ids.map(() => '(?, ?, ?, ?)').join(', ') + const params = ids.flatMap((streamId) => [userId, streamId, channel, 'off']) + return query( + `INSERT INTO notification_channel_prefs (user_id, stream_id, channel, mode) + VALUES ${values} + ON DUPLICATE KEY UPDATE mode = VALUES(mode)`, + params, + ) +} + +module.exports = { listByUser, upsert, offPushExcept, offForChannel } diff --git a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js index 1aeff0b..18e5c20 100644 Binary files a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js and b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js differ diff --git a/server/src/model/teams/teamNotify.db.js b/server/src/model/teams/teamNotify.db.js index 7beb8ee..32a8f2d 100644 --- a/server/src/model/teams/teamNotify.db.js +++ b/server/src/model/teams/teamNotify.db.js @@ -211,11 +211,31 @@ async function digestPostsSince(teamId, since, limit = 20) { ) } +/** + * The preference rows for a set of users in one Team — the scope-preference + * provider's only query (ENGAGEMENT.md Phase 6). + * + * Returns only the rows that EXIST. Absence is answered by the caller, which is + * the same discipline the two recipient queries follow with their COALESCEs: the + * default lives in one place and it is the schema. + */ +async function prefsForTeam(userIds, teamId) { + const ids = userIds.filter(isUserId) + if (!ids.length) return [] + return query( + `SELECT user_id, muted, email_mode + FROM team_notification_prefs + WHERE team_id = ? AND user_id IN (${ids.map(() => '?').join(',')})`, + [teamId, ...ids], + ) +} + module.exports = { recipientIds, emailRecipients, prefsForUser, prefFor, + prefsForTeam, setPref, stampDigest, teamsWithForumActivitySince, diff --git a/server/src/model/teams/teamNotify.model.js b/server/src/model/teams/teamNotify.model.js index 6f5d70f..32397b1 100644 --- a/server/src/model/teams/teamNotify.model.js +++ b/server/src/model/teams/teamNotify.model.js @@ -140,6 +140,8 @@ module.exports = { // time, so the boundary is real. recipientIds: (teamId, opts) => db.recipientIds(teamId, opts), emailRecipients: (teamId, opts) => db.emailRecipients(teamId, opts), + prefsForTeam: (userIds, teamId) => db.prefsForTeam(userIds, teamId), + setEmailMode: (userId, teamId, emailMode) => db.setPref(userId, teamId, { emailMode }), stampDigest: (userId, teamId, at) => db.stampDigest(userId, teamId, at), teamsWithForumActivitySince: (since) => db.teamsWithForumActivitySince(since), digestPostsSince: (teamId, since, limit) => db.digestPostsSince(teamId, since, limit), diff --git a/server/src/model/teams/teamSync.model.js b/server/src/model/teams/teamSync.model.js index 56e9cd2..90141cf 100644 --- a/server/src/model/teams/teamSync.model.js +++ b/server/src/model/teams/teamSync.model.js @@ -217,8 +217,23 @@ async function notifyRoster(team, { joined, promoted, demoted }) { // The count rides along for the Discord bridge (§7.2), which has no app on // the other end to pull the roster after a content-free nudge. The tickle // itself is unchanged and still carries nothing. - if (joined.length > 0) await teamNotify.memberJoined(team, { count: joined.length }) - if (promoted.length > 0 || demoted.length > 0) await teamNotify.leadershipChanged(team) + // `names` is the engagement engine's half (ENGAGEMENT.md Phase 6): the two + // triggers declare `memberName` / `leaderName` as required single values, so + // the fan-out emits one event per person while the tickle and the bridge stay + // one per run. A member the module reported without a display name is skipped + // rather than emitted as "someone" — a required variable filled with a + // placeholder is a mail that names nobody. + if (joined.length > 0) { + await teamNotify.memberJoined(team, { + count: joined.length, + names: joined.map((m) => m.display_name).filter(Boolean), + }) + } + if (promoted.length > 0 || demoted.length > 0) { + await teamNotify.leadershipChanged(team, { + names: promoted.map((m) => m.display_name).filter(Boolean), + }) + } } catch (err) { log.warn('roster notification not sent', { teamId: team.id, message: err.message }) } diff --git a/server/src/router/v1/public/engagement.controller.js b/server/src/router/v1/public/engagement.controller.js new file mode 100644 index 0000000..a69f4a8 --- /dev/null +++ b/server/src/router/v1/public/engagement.controller.js @@ -0,0 +1,108 @@ +// ── Public engagement surface: one-click unsubscribe ─────────────────────── +// +// ENGAGEMENT.md Phase 6. This is the generalization of what +// `public/teams.controller.js` did for Teams: a token names a CHANNEL and a +// SCOPE, and honouring it turns that channel off for that scope. +// +// **The old path stays forever**, and that is not tidiness debt. A link in a mail +// sent before this deploy points at `/public/teams/unsubscribe/:token`, and mail +// is not editable after it has been sent; a route that moves is a person who +// cannot unsubscribe. `teams.router.js` therefore keeps its two routes and hands +// them straight to these handlers, so the two paths cannot drift into meaning +// different things. + +const teamPrefs = require('../../../model/teams/teamNotify.model') +const prefs = require('../../../model/notificationChannelPrefs/notificationChannelPrefs.model') +const unsubscribeToken = require('../../../utils/unsubscribeToken') +const scopedPrefs = require('../../../engagement/scopedPrefs') +const log = require('../../../utils/logger')('engagement') + +/** + * Apply one verified claim. + * + * **Scoped claims are written to the scope's own store, not to + * `notification_channel_prefs`.** A scoped preference is what the engine reads + * for a scoped event (engine.js `effectiveModes`), so writing 'off' anywhere else + * would be an unsubscribe that changes a row nothing consults. Today `team` is + * the only registered scope, and it is handled here rather than through a + * registry write-back for the reason Phase 3 gave for deferring `deliver`: a + * second scope is what should design that interface, not the first one. + * + * An UNSCOPED claim (`scopeKey === ''`) turns the channel off across the board — + * which today no mail produces, because every mail this platform sends carries a + * scope. It is implemented rather than refused so that the first deployment-wide + * mail does not ship with an unsubscribe link that quietly does nothing. + */ +async function applyClaim(claim) { + if (!claim.scopeKey) { + await prefs.setAllChannelOff(claim.userId, claim.channel) + return + } + const parsed = scopedPrefs.parse(claim.scopeKey) + if (parsed && parsed.prefix === 'team') { + if (claim.channel === 'email') { + // **Not `mute`, and this is Phase 6's deliberate narrowing.** A v1 token set + // `muted = 1`, which silenced that Team's push as well as its email — a link + // labelled "stop these emails" quietly stopping notifications on somebody's + // phone. A token now names its channel and turns off that channel only. + await teamPrefs.setEmailMode(claim.userId, Number(parsed.id), 'off') + return + } + await teamPrefs.mute(claim.userId, Number(parsed.id)) + return + } + // A scope whose provider is not registered — a module uninstalled since the + // mail went out. Nothing to write, and the caller is still told 200: the mail + // that named it cannot be sent again either. + log.warn('unsubscribe named an unknown scope', { scope: claim.scopeKey }) +} + +/** + * POST /public/engagement/unsubscribe/:token — one-click unsubscribe (RFC 8058). + * + * **The one write in this tier, and it is unauthenticated on purpose.** A person + * reading their mail is not logged into the site, and an unsubscribe that first + * demands a login is an unsubscribe most people do not complete. The token is what + * stands in for the session, and the capability it carries is deliberately the + * narrowest one that does the job: turn ONE channel off for ONE scope. It reads + * nothing, cannot turn anything back on, and names no other scope. + * + * **Always 200, whatever the token was.** A response that distinguished a valid + * token from a forged one would turn this into an oracle for which (user, scope) + * pairs exist, on an endpoint with no session behind it. The page says "you will + * not receive further emails about this" either way, which is true either way. + * + * Reached two ways with the same effect: a mail client's RFC 8058 one-click POST + * (the `List-Unsubscribe-Post` header), and the site's own /unsubscribe page, + * which POSTs here after a human clicks the link in the body. + */ +async function unsubscribe(req, res) { + const claim = unsubscribeToken.verify(req.params.token) + if (claim) { + try { + await applyClaim(claim) + } catch (err) { + // Logged, not surfaced. A failed write here is worth an operator's + // attention and is not worth telling an anonymous caller about — and a 500 + // would make a mail client retry a request it should not repeat. + log.error('unsubscribe', err) + } + } + return res.json({ ok: true }) +} + +/** + * GET on the same path — for a mail client that shows the `List-Unsubscribe` URL + * as a link and has no one-click support. + * + * Redirects to the site's own page rather than acting, because a GET must not + * mutate: a link prefetcher or a mail client's link scanner would otherwise + * silently unsubscribe people who asked for nothing. The page it lands on does the + * POST once a human is looking at it. + */ +function unsubscribeLanding(req, res) { + const base = (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') + return res.redirect(302, `${base}/unsubscribe/${encodeURIComponent(req.params.token)}`) +} + +module.exports = { unsubscribe, unsubscribeLanding, applyClaim } diff --git a/server/src/router/v1/public/engagement.router.js b/server/src/router/v1/public/engagement.router.js new file mode 100644 index 0000000..aba2989 --- /dev/null +++ b/server/src/router/v1/public/engagement.router.js @@ -0,0 +1,39 @@ +const express = require('express') + +const ctrl = require('./engagement.controller') + +const engagementRouter = express.Router() + +// ── One-click unsubscribe (ENGAGEMENT.md Phase 6) ────────────────────────── +// +// The canonical home of the unsubscribe pair, generalized off +// `/public/teams/unsubscribe/:token`. That path still exists and still works — +// see `teams.router.js` — because links in mail already sent cannot be rewritten. +// +// No `siteMode`, unlike almost every other public route. An unsubscribe has to +// work while the site is in maintenance: the mail that carried the link went out +// before the site went down, and "we are doing maintenance" is not an answer to +// "stop emailing me". +engagementRouter.post( + '/unsubscribe/:token', + // #swagger.tags = ['Public · Engagement'] + // #swagger.summary = 'Unsubscribe from one channel for one scope' + // #swagger.description = 'Honours the tokened link in an engagement email, including RFC 8058 one-click. The token names a delivery channel and a scope; the write turns that channel off for that scope and nothing else. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, scope) pairs exist. Tokens signed before this route existed are still honoured, at this path and at the older /public/teams one.' + // #swagger.parameters['token'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The signed token from the email link.' } + // #swagger.security = [{}] + /* #swagger.responses[200] = { description: 'Acknowledged', content: { "application/json": { schema: { $ref: "#/components/schemas/OkFlag" } } } } */ + ctrl.unsubscribe, +) + +engagementRouter.get( + '/unsubscribe/:token', + // #swagger.tags = ['Public · Engagement'] + // #swagger.summary = 'Land a human on the unsubscribe page' + // #swagger.description = 'For mail clients that render the List-Unsubscribe URL as an ordinary link. Redirects to the site’s own confirmation page and changes nothing — a GET must not mutate, or a link scanner would unsubscribe people who asked for nothing.' + // #swagger.parameters['token'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The signed token from the email link.' } + // #swagger.security = [{}] + /* #swagger.responses[302] = { description: 'Redirect to the site’s unsubscribe page' } */ + ctrl.unsubscribeLanding, +) + +module.exports = engagementRouter diff --git a/server/src/router/v1/public/index.js b/server/src/router/v1/public/index.js index fb7f636..139a8a0 100644 --- a/server/src/router/v1/public/index.js +++ b/server/src/router/v1/public/index.js @@ -22,6 +22,7 @@ const wikiRouter = require('./wiki.router') const pagesRouter = require('./pages.router') const modulesRouter = require('./modules.router') const teamsRouter = require('./teams.router') +const engagementRouter = require('./engagement.router') const siteRouter = require('./site.router') const publicRouter = express.Router() @@ -41,6 +42,12 @@ publicRouter.use('/modules', modulesRouter) // is what populates it (TEAMS.md §10.3). Site-mode gated per route, like the // content above it. publicRouter.use('/teams', teamsRouter) +// The unauthenticated half of the engagement system: today exactly the +// unsubscribe pair. Its own prefix rather than a Teams sub-path, because what a +// token names is a channel and a scope and a scope is not always a Team +// (ENGAGEMENT.md Phase 6). Never site-mode gated — an unsubscribe has to work +// while the site is in maintenance. +publicRouter.use('/engagement', engagementRouter) // The four singletons that own no path segment of their own: /settings, /status, // /version and /contact. Mounted at the group root, last — safe only because diff --git a/server/src/router/v1/public/teams.controller.js b/server/src/router/v1/public/teams.controller.js index b41825f..6ff6b7f 100644 --- a/server/src/router/v1/public/teams.controller.js +++ b/server/src/router/v1/public/teams.controller.js @@ -6,8 +6,6 @@ const teams = require('../../../model/teams/teams.model') const teamActivity = require('../../../model/teams/teamActivity.model') -const teamPrefs = require('../../../model/teams/teamNotify.model') -const unsubscribeToken = require('../../../utils/unsubscribeToken') const log = require('../../../utils/logger')('teams') @@ -100,52 +98,18 @@ async function getActivity(req, res) { } } -/** - * POST /public/teams/unsubscribe/:token — one-click unsubscribe (TEAMS.md §6.4). - * - * **The one write in this tier, and it is unauthenticated on purpose.** A person - * reading their mail is not logged into the site, and an unsubscribe that first - * demands a login is an unsubscribe most people do not complete. The token is what - * stands in for the session, and the capability it carries is deliberately the - * narrowest one that does the job: set `muted` for ONE (user, Team) pair. It reads - * nothing, cannot un-mute, and names no other Team. - * - * **Always 200, whatever the token was.** A response that distinguished a valid - * token from a forged one would turn this into an oracle for which (user, Team) - * pairs exist, on an endpoint with no session behind it. The page says "you will - * not receive further emails about this team" either way, which is true either way. - * - * Reached two ways with the same effect: a mail client's RFC 8058 one-click POST - * (the `List-Unsubscribe-Post` header), and the site's own /unsubscribe page, - * which POSTs here after a human clicks the link in the body. - */ -async function unsubscribe(req, res) { - const claim = unsubscribeToken.verify(req.params.token) - if (claim) { - try { - await teamPrefs.mute(claim.userId, claim.teamId) - } catch (err) { - // Logged, not surfaced. A failed write here is worth an operator's - // attention and is not worth telling an anonymous caller about — and a 500 - // would make a mail client retry a request it should not repeat. - log.error('unsubscribe', err) - } - } - return res.json({ ok: true }) -} - -/** - * GET on the same path — for a mail client that shows the `List-Unsubscribe` URL - * as a link and has no one-click support. - * - * Redirects to the site's own page rather than acting, because a GET must not - * mutate: a link prefetcher or a mail client's link scanner would otherwise - * silently mute Teams nobody asked to leave. The page it lands on does the POST - * once a human is looking at it. - */ -function unsubscribeLanding(req, res) { - const base = (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') - return res.redirect(302, `${base}/unsubscribe/${encodeURIComponent(req.params.token)}`) -} +// ── One-click unsubscribe: the legacy path ───────────────────────────────── +// +// The handlers moved to `engagement.controller.js` in ENGAGEMENT.md Phase 6, +// because what a token names is a channel and a scope and a scope is not always a +// Team. **This path did NOT move**, and cannot: every Team notification sent +// before that phase carries `/public/teams/unsubscribe/` in its +// `List-Unsubscribe` header and in its body, mail is not editable once sent, and +// a route that moves is a person who cannot unsubscribe. +// +// Re-exported rather than reimplemented, so the two paths cannot drift into +// meaning different things. A v1 token arriving here reads as +// `{ channel: 'email', scopeKey: 'team:' }` — see unsubscribeToken's header. +const { unsubscribe, unsubscribeLanding } = require('./engagement.controller') module.exports = { listTeams, getTeam, getTeamByExternalId, getRoster, getActivity, unsubscribe, unsubscribeLanding } diff --git a/server/src/router/v1/public/teams.router.js b/server/src/router/v1/public/teams.router.js index 118ae81..ac4502f 100644 --- a/server/src/router/v1/public/teams.router.js +++ b/server/src/router/v1/public/teams.router.js @@ -90,13 +90,20 @@ teamsRouter.get( ctrl.getActivity, ) -// ── One-click unsubscribe (TEAMS.md §6.4) ────────────────────────────────── +// ── One-click unsubscribe — the LEGACY path (TEAMS.md §6.4) ──────────────── +// +// The canonical pair now lives at `/public/engagement/unsubscribe/:token` +// (ENGAGEMENT.md Phase 6). These two stay, permanently, and hand straight to the +// same handlers: mail sent before that phase carries this path in its +// `List-Unsubscribe` header, and a route that moves is a person who cannot +// unsubscribe. // // Declared last, and the shadowing question is worth answering rather than // assuming: these are two segments, so the one-segment '/:slug' cannot take them, // and the two-segment '/:slug/members' and '/:slug/activity' both pin a LITERAL // second segment. Only a token spelled exactly "members" or "activity" could -// collide, and a token is `...`. +// collide, and a token is `...` (v1) or +// `....` (v2). // // No `siteMode`, unlike every other route in this file. An unsubscribe has to work // while the site is in maintenance: the mail that carried the link went out before @@ -105,8 +112,8 @@ teamsRouter.get( teamsRouter.post( '/unsubscribe/:token', // #swagger.tags = ['Public · Teams'] - // #swagger.summary = 'Unsubscribe from one Team’s notification emails' - // #swagger.description = 'Honours the tokened link in a Team notification email, including RFC 8058 one-click. Sets the same per-Team mute the account screen shows. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.' + // #swagger.summary = 'Unsubscribe from one Team’s notification emails (legacy path)' + // #swagger.description = 'The pre-Phase-6 path, kept permanently because links in mail already sent point at it. Identical to POST /public/engagement/unsubscribe/{token}. Honours the tokened link including RFC 8058 one-click; a token signed before Phase 6 turns off that Team’s email and no longer mutes its push. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.' // #swagger.parameters['token'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The signed token from the email link.' } // #swagger.security = [{}] /* #swagger.responses[200] = { description: 'Acknowledged', content: { "application/json": { schema: { $ref: "#/components/schemas/OkFlag" } } } } */ diff --git a/server/src/utils/engagementEmit.js b/server/src/utils/engagementEmit.js index e3e2262..0021bef 100644 --- a/server/src/utils/engagementEmit.js +++ b/server/src/utils/engagementEmit.js @@ -22,6 +22,7 @@ const registries = require('../modules/registries') const engine = require('../engagement/engine') +const scopedPrefs = require('../engagement/scopedPrefs') const createLogger = require('./logger') const log = createLogger('engagement') @@ -39,6 +40,14 @@ const RELATIVE_URL = /^\/(?!\/)[A-Za-z0-9\-._~/?#[\]@!$&'()*+,;=%]*$/ // to prevent. const DEDUPE_KEY_MAX = 190 +// engagement_outbox.scope_key is VARCHAR(190), same reasoning as above. +const SCOPE_KEY_MAX = 190 + +// An emitter asserting an audience asserts a BOUNDED one. `MAX_AUDIENCE` (5000) +// already caps what the engine will load from a query; this is the matching bound +// on a list a caller built itself, and it is the same number for the same reason. +const RECIPIENTS_MAX = 5000 + const isProd = () => process.env.NODE_ENV === 'production' /** Coerce and check one declared variable. Returns `{ value }` or `{ error }`. */ @@ -156,7 +165,7 @@ function emit(owner, triggerId, envelope = {}) { return fail(`"${triggerId}" is kind "${declaration.kind}" and is not emitted directly`) } - const { subject, data, ownerUserId, dedupeKey, occurredAt } = envelope || {} + const { subject, data, ownerUserId, dedupeKey, occurredAt, scopeKey, recipientUserIds } = envelope || {} const payload = validatePayload(declaration, data) if (!payload.ok) return fail(`payload for "${triggerId}" is invalid`, payload.errors.join('; ')) @@ -181,6 +190,41 @@ function emit(owner, triggerId, envelope = {}) { } } + // The scope this event is ABOUT: `team:12`, or absent. Distinct from `subject`, + // which is what a cooldown counts — see the engine's enqueue. It is a stable + // identifier because an unsubscribe token is signed over it and sits in a + // mailbox for months; a display name would orphan the link on the first rename. + let resolvedScope = null + if (scopeKey !== undefined && scopeKey !== null) { + if (typeof scopeKey !== 'string' || !scopedPrefs.parse(scopeKey)) { + return fail('scopeKey must be a string of the form ":"') + } + if (scopeKey.length > SCOPE_KEY_MAX) return fail(`scopeKey must be at most ${SCOPE_KEY_MAX} characters`) + resolvedScope = scopeKey + } + + // **The audience this particular firing is about** (Phase 6, decision 2). An + // emitter that has already computed an access-checked recipient set — the Team + // fan-out is the case that forced it — hands it over here, and a rule whose + // audience is `members` resolves to it. It is a NARROWING input, not a + // widening one: `audiences.resolveForRule` still filters it through + // `users.status`, the ceiling is still `members`, and the G24 check still runs. + // A rule with any other audience ignores it entirely. + let resolvedRecipients = null + if (recipientUserIds !== undefined && recipientUserIds !== null) { + if (!Array.isArray(recipientUserIds)) return fail('recipientUserIds must be an array') + if (recipientUserIds.length > RECIPIENTS_MAX) { + // Bounded here rather than at the query, because the bound is about what an + // emitter may assert. `MAX_AUDIENCE` already caps what the engine will load; + // this stops a caller building a list that large in the first place. + return fail(`recipientUserIds must hold at most ${RECIPIENTS_MAX} ids`) + } + if (!recipientUserIds.every((n) => Number.isInteger(n) && n > 0)) { + return fail('recipientUserIds must be positive integers') + } + resolvedRecipients = [...new Set(recipientUserIds)] + } + if (dedupeKey !== undefined && dedupeKey !== null) { if (typeof dedupeKey !== 'string' || !dedupeKey || dedupeKey.length > DEDUPE_KEY_MAX) { return fail(`dedupeKey must be a string of 1-${DEDUPE_KEY_MAX} characters`) @@ -200,6 +244,8 @@ function emit(owner, triggerId, envelope = {}) { version: declaration.version, subject: resolvedSubject, ownerUserId: ownerUserId === undefined ? null : ownerUserId, + scopeKey: resolvedScope, + recipientUserIds: resolvedRecipients, dedupeKey: dedupeKey === undefined ? null : dedupeKey, occurredAt: at.toISOString(), data: payload.data, @@ -233,4 +279,4 @@ function emit(owner, triggerId, envelope = {}) { return { ok: true, event } } -module.exports = { emit, validatePayload, RELATIVE_URL, DEDUPE_KEY_MAX } +module.exports = { emit, validatePayload, RELATIVE_URL, DEDUPE_KEY_MAX, SCOPE_KEY_MAX, RECIPIENTS_MAX } diff --git a/server/src/utils/engagementWorker.js b/server/src/utils/engagementWorker.js index bea3771..d486f13 100644 --- a/server/src/utils/engagementWorker.js +++ b/server/src/utils/engagementWorker.js @@ -56,7 +56,7 @@ const STALE_MS = 15 * 60 * 1000 /** * Deliver one claimed row. * - * @returns {{ outcome: 'sent'|'retry'|'terminal', detail?: string, transport?: string }} + * @returns {{ outcome: 'sent'|'retry'|'terminal', detail?: string, transport?: string, addressHash?: string }} */ async function deliver(row) { const channel = channels.get(row.channel) @@ -71,9 +71,17 @@ async function deliver(row) { } try { const result = await channel.deliver(row) - if (result && result.ok) return { outcome: 'sent', transport: result.transport, detail: result.detail } - if (result && result.retry) return { outcome: 'retry', detail: result.detail || 'transient failure' } - return { outcome: 'terminal', detail: (result && result.detail) || 'delivery refused' } + // `addressHash` rides on every outcome, success or not: a bounce (Phase 9) + // arrives with an address and has to find the row it belongs to, and the rows + // worth correlating include the ones that already failed once. + const hash = (result && result.addressHash) || undefined + if (result && result.ok) { + return { outcome: 'sent', transport: result.transport, detail: result.detail, addressHash: hash } + } + if (result && result.retry) { + return { outcome: 'retry', detail: result.detail || 'transient failure', addressHash: hash } + } + return { outcome: 'terminal', detail: (result && result.detail) || 'delivery refused', addressHash: hash } } catch (err) { // A channel shouldn't throw, but if one does it is a transient failure // rather than a crashed tick - announceWorker's posture with its legs. @@ -111,6 +119,7 @@ async function processRow(row, now = new Date(), deliverFn = deliver) { user_id: row.user_id, channel: row.channel, transport: result.transport ?? null, + address_hash: result.addressHash ?? null, status, detail: result.detail ?? null, }) diff --git a/server/src/utils/mailer.js b/server/src/utils/mailer.js index 8ec3870..bdd3a5d 100644 --- a/server/src/utils/mailer.js +++ b/server/src/utils/mailer.js @@ -20,9 +20,15 @@ // contracts are unchanged by the transport rewrite and are asserted in // test/mailer.test.js: sendContactMessage returns a mailto fallback, sendInvite // and sendPasswordReset return { sent: false, reason: 'NOT_CONFIGURED' } so their -// callers can surface a link / answer a generic 200, sendTeamNotification never -// throws at all, and only sendTest throws — because only sendTest has an admin -// waiting to be told why. +// callers can surface a link / answer a generic 200, sendNotification classifies +// its failure for a worker rather than reporting it to anybody, and only sendTest +// throws — because only sendTest has an admin waiting to be told why. +// +// Phase 6 removed `sendTeamNotification`. It was the last sender that built its +// own body shape, and what replaced it is not another sender: the email +// DeliveryChannel renders an `engagement_templates` row and calls +// `sendNotification`, so a Team mail is now the same kind of thing as any other +// rule-driven mail. const emailConfig = require('../model/emailConfig/emailConfig.model') const settings = require('../model/settings/settings.model') @@ -338,77 +344,6 @@ async function sendEmailVerification({ to, verifyUrl, username }) { } } -/** - * Send a Team notification — one event (`immediate` mode) or a day's worth - * (`digest` mode). TEAMS.md §6.4. - * - * **This one carries CONTENT, and the push tickle beside it deliberately does - * not.** A tickle goes to ntfy, an untrusted relay reachable by an unguessable - * topic, so it carries `{ stream, ref }` and the app pulls the real thing over an - * access-checked API. A mailbox is a destination the recipient chose. Same - * reasoning as the Discord bridge (§7.2), and it is why this function takes - * excerpts rather than ids. - * - * **Excerpts, never full posts.** Partly courtesy, mostly so that the blast radius - * of a mis-addressed or forwarded mail is a sentence rather than a thread. The - * caller does the truncation, because it is the caller that knows the body was - * already stripped of markup. - * - * The `List-Unsubscribe` pair is what makes a mail client's own unsubscribe button - * appear, and both halves are needed: the `mailto:`-free URL form for clients that - * open the link, and `List-Unsubscribe-Post` for RFC 8058 one-click, which POSTs - * without ever showing the user a page. Both reach the same tokened endpoint that - * writes the same per-Team mute the site shows. - * - * Never throws. A notification failing must not fail the forum write that caused - * it, and there is nobody up the stack to catch it — the digest worker runs on a - * timer and the immediate send is fired from a request that has already replied. - */ -async function sendTeamNotification({ to, subject, intro, items, teamUrl, unsubscribeUrl, unsubscribeApiUrl }) { - const built = await buildTransport() - if (!built) return { sent: false, reason: 'NOT_CONFIGURED' } - const { transport, config } = built - - const lines = [intro, ''] - for (const item of items || []) { - lines.push(`${item.heading}`) - if (item.excerpt) lines.push(` ${item.excerpt}`) - if (item.url) lines.push(` ${item.url}`) - lines.push('') - } - if (teamUrl) lines.push(teamUrl, '') - if (unsubscribeUrl) { - lines.push('To stop these emails for this team, use this link:', unsubscribeUrl) - } - - try { - await transport.sendMail({ - from: fromHeader(config), - to, - replyTo: replyToFor(config), - subject, - text: lines.join('\n'), - // The header carries the API url, not the one in the body: a one-click - // client POSTs to whatever is here without rendering anything, so it has to - // be an endpoint. Falls back to the body's url when no API one was passed. - headers: (unsubscribeApiUrl || unsubscribeUrl) - ? { - 'List-Unsubscribe': `<${unsubscribeApiUrl || unsubscribeUrl}>`, - 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', - } - : undefined, - }) - return { sent: true } - } catch (err) { - // Logged and swallowed, unlike every other sender in this file. Those are - // called by a request that can report the failure to whoever caused it; this - // one is not, and recordStatus already puts the error where an admin reads it. - log.warn('team notification send failed', { message: err.message }) - await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err, config) }).catch(() => {}) - return { sent: false, reason: 'SEND_FAILED' } - } -} - /** * Send an ALREADY-RENDERED body to one address — the template editor's test send * (§4.6.2, Phase 5b). @@ -451,13 +386,77 @@ async function sendRendered(to, rendered) { } } +// A send failure that will still be a failure on the fifth attempt. Everything +// else — a refused connection, a timeout, a relay having a bad minute, a +// deployment whose operator is halfway through typing its credentials — is worth +// the flat five-minute retry `engagementWorker` gives it. Getting this backwards +// in the safe direction costs four pointless reconnects; getting it backwards in +// the other direction drops somebody's mail on a transient blip. +const PERMANENT_CODES = new Set([550, 553, 554, 'EENVELOPE', 'EAUTH']) + +/** + * Deliver one already-rendered engagement message (ENGAGEMENT.md Phase 6). + * + * The third sender in this file that never throws, and the reasons are the three + * different ones: `sendContactMessage` reports to a request, `sendTest` to an + * admin standing at a button, and this one to a worker sweeping a queue at three + * in the morning. What it returns is a CLASSIFICATION rather than a boolean, + * because the worker's next move — retry, or write a terminal row in the send + * log — is exactly what a boolean cannot say. + * + * **It does not `recordStatus('connected')` on success**, unlike `sendRendered`. + * That column is the admin screen's account of whether the operator's + * configuration works, written by the actions an operator takes; a background + * sweep quietly flipping it to "Test send OK" would be this file reporting on + * itself. A FAILURE is still recorded, because a relay that has started refusing + * mail is precisely what that screen exists to show. + * + * @returns {Promise<{ok: boolean, retry?: boolean, transport?: string, detail?: string}>} + */ +async function sendNotification({ to, rendered, unsubscribeUrl, unsubscribeApiUrl }) { + const built = await buildTransport() + // Retryable, not terminal: an operator midway through setting up SMTP should + // find the queue drains rather than a backlog of permanently failed rows. + if (!built) return { ok: false, retry: true, detail: 'email is not configured' } + const { transport, config } = built + + try { + await transport.sendMail({ + from: fromHeader(config), + to, + replyTo: replyToFor(config), + subject: rendered.subject, + text: rendered.text, + html: rendered.html, + // The header carries the API url, not the one in the body: a one-click + // client POSTs to whatever is here without rendering anything, so it has to + // be an endpoint. Both headers or neither — RFC 8058 one-click is only + // one-click when the POST variant says so. + headers: (unsubscribeApiUrl || unsubscribeUrl) + ? { + 'List-Unsubscribe': `<${unsubscribeApiUrl || unsubscribeUrl}>`, + 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', + } + : undefined, + }) + return { ok: true, transport: config.transport } + } catch (err) { + const detail = describeSendError(err, config) + const code = err && (err.responseCode || err.code) + log.warn('engagement send failed', { message: err.message }) + await emailConfig.recordStatus({ status: 'error', statusDetail: detail }).catch(() => {}) + return { ok: false, retry: !PERMANENT_CODES.has(code), transport: config.transport, detail } + } +} + module.exports = { isConfigured, sendContactMessage, sendTest, sendRendered, + sendNotification, sendInvite, sendPasswordReset, sendEmailVerification, - sendTeamNotification, + PERMANENT_CODES, } diff --git a/server/src/utils/teamDigestWorker.js b/server/src/utils/teamDigestWorker.js index 50d5c74..28cf176 100644 --- a/server/src/utils/teamDigestWorker.js +++ b/server/src/utils/teamDigestWorker.js @@ -1,13 +1,14 @@ -// ── Team forum digest worker (TEAMS.md §6.4, phase 6) ────────────────────── +// ── Team forum digest worker (TEAMS.md §6.4; migrated in ENGAGEMENT.md Phase 6) ── // // Daily, per (user, Team): "here is what you missed". The same in-process shape as // utils/teamActivityPrune and utils/announceWorker — setInterval + unref + stop(), // wired into server.js start/shutdown. There is no cron in this stack. // -// **It computes at send time and keeps no queue.** The only state is -// `team_notification_prefs.last_digest_at`; everything else is re-derived from the -// forum tables when the mail is about to go out. Three properties fall out of that, -// and they are why the design chose it over a pending-items table: +// **It computes at send time and keeps no queue, and Phase 6 deliberately did not +// change that.** §4.2b's decision was to generalize this worker's STATE, not its +// absence of one. Three properties fall out of computing at send time, and they +// are why the design chose it over a pending-items table — and, now, over the +// engine's own outbox: // // 1. A deployment that was down for two days sends ONE correct digest, not two // days of replay. @@ -18,14 +19,29 @@ // the recipient set, so they are not emailed content they can no longer read. // This is the one that would have been a security bug. // +// An outbox row carries a snapshot of the payload taken at emit time and has none +// of the three. That is why `engine.subscribedTo` enqueues only `instant` +// recipients and leaves `digest` to this file. +// +// **What DID change is everything around the query.** The state is +// `engagement_digest_state` rather than a column on the preferences row; the body +// is the `notify.digest` template an operator can edit rather than a literal in +// `mailer`; the unsubscribe link is a v2 token naming the email channel; and the +// whole worker is gated on an ENABLED rule, so an operator who switches Team +// email off switches off both halves of it rather than the immediate half only. +// // **The first run is delayed, for the same reason the prune's is**: a boot that is // crash-looping must not send mail on every loop. const teamNotify = require('../model/teams/teamNotify.model') const forumSettings = require('../model/teams/teamForumSettings.model') +const rulesDb = require('../model/engagement/engagementRules.db') +const sendsDb = require('../model/engagement/engagementSends.db') +const digestDb = require('../model/engagement/engagementDigest.db') +const templates = require('../engagement/templates') +const emailChannel = require('../engagement/emailChannel') const mailer = require('./mailer') const notify = require('./teamNotify') -const brand = require('../config/brand') const log = require('./logger')('team-digest') const INTERVAL_MS = Number(process.env.TEAM_DIGEST_INTERVAL_MS) || 24 * 60 * 60 * 1000 @@ -41,6 +57,16 @@ const MAX_LOOKBACK_MS = 7 * 24 * 60 * 60 * 1000 // to the Team is the better answer. const MAX_ITEMS = 20 +// The two triggers a Team forum digest summarises. A rule on either, enabled and +// naming the email channel, is what turns this worker on. +const DIGEST_TRIGGERS = ['team.forum.post', 'team.announcement'] + +// The digest's own template key, and the rule's `template_keys.digest` overrides +// it. A digest is not the same message as the instant mail and must not silently +// borrow `template_keys.email`: that template is written for one event and would +// render a day's worth of posts as a single missing `{{threadTitle}}`. +const DEFAULT_TEMPLATE = 'notify.digest' + let timer = null let firstRun = null @@ -53,6 +79,29 @@ const clampSince = (last, now) => { return at < floor ? floor : at } +/** + * The enabled email rule that authorises Team forum digests, or null. + * + * **The gate is why this worker did not simply keep running.** Phase 6's decision + * 3 is that Team notifications become rules an operator turns on; if the immediate + * mail were rule-gated and the digest were not, disabling the rule would stop one + * kind of Team mail and leave a daily summary arriving indefinitely — which reads + * as the switch being broken. + * + * The FIRST matching rule wins, and the tie-break is not interesting because what + * is read off it is one template key. Two rules disagreeing about the template of + * a digest neither of them describes is a configuration an operator can see in the + * rules list. + */ +async function digestRule() { + for (const triggerId of DIGEST_TRIGGERS) { + const rules = await rulesDb.enabledForTrigger(triggerId) + const rule = rules.find((r) => (r.channels || []).includes('email')) + if (rule) return rule + } + return null +} + /** * One recipient's digest for one Team. Returns true if a mail went out. * @@ -61,7 +110,8 @@ const clampSince = (last, now) => { * stamping first, silently eats a day of somebody's notifications every time the * mail provider has a bad minute. */ -async function sendOne(team, recipient, now) { +async function sendOne(team, recipient, rule, now) { + const scopeKey = notify.scopeKey(team) const since = clampSince(recipient.last_digest_at, now) const posts = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS) // Nothing new for THIS recipient — which is not the same as nothing new for the @@ -70,21 +120,54 @@ async function sendOne(team, recipient, now) { if (posts.length === 0) return false const label = notify.teamLabel(team) - const res = await mailer.sendTeamNotification({ - to: recipient.email, - subject: `[${brand.name}] ${label}: ${posts.length} new post${posts.length === 1 ? '' : 's'}`, + const unsub = emailChannel.unsubscribeUrls(recipient.user_id, scopeKey) + const key = (rule && rule.template_keys && rule.template_keys.digest) || DEFAULT_TEMPLATE + + const rendered = await templates.renderByKey(key, { + periodLabel: `${label}: ${posts.length} new post${posts.length === 1 ? '' : 's'}`, intro: `Since your last digest, ${posts.length} new post${posts.length === 1 ? '' : 's'} in ${label}:`, items: posts.map((p) => ({ heading: `${p.title} — ${p.author_username || 'someone'}`, excerpt: notify.excerpt(p.body_html), - url: notify.threadUrl(team, p.thread_id), + url: notify.threadPath(team, p.thread_id), })), - teamUrl: notify.teamPageUrl(team), - unsubscribeUrl: notify.unsubscribeUrl(recipient.user_id, team.id), - unsubscribeApiUrl: notify.unsubscribeApiUrl(recipient.user_id, team.id), + scopeUrl: notify.teamPagePath(team), + ...unsub, }) - if (!res || !res.sent) return false - await teamNotify.stampDigest(recipient.user_id, team.id, now) + if (!rendered) { + log.warn('digest template is missing and has no shipped default', { key }) + return false + } + + const result = await mailer.sendNotification({ to: recipient.email, rendered, ...unsub }) + + // Recorded in `engagement_sends` like every other message the platform sends, + // which is G15's whole point: "did user X get the digest?" was unanswerable + // before this phase because the digest went out through a sender that wrote + // nothing down. `outbox_id` is null because a digest has no outbox row — see + // this file's header — and that null is the honest record of a different path, + // not a missing value. + await sendsDb + .record({ + outbox_id: null, + rule_id: rule ? rule.id : null, + trigger_id: DIGEST_TRIGGERS[0], + user_id: recipient.user_id, + channel: 'email', + transport: result.transport ?? null, + // The same hash the instant path writes, and it has to be the same + // function: a bounce (Phase 9) arrives with an address and is matched + // against this column, so a digest row without one is a delivery that + // cannot be correlated. Found by reading the send log on the live rig, + // where the instant row had a hash and the digest row beside it did not. + address_hash: emailChannel.hashAddress(recipient.email), + status: result.ok ? 'sent' : 'failed', + detail: result.ok ? null : result.detail, + }) + .catch((err) => log.warn('digest send not logged', { message: err.message })) + + if (!result.ok) return false + await digestDb.stamp(recipient.user_id, 'email', scopeKey, now) return true } @@ -97,11 +180,14 @@ async function sendOne(team, recipient, now) { async function tick(now = new Date()) { const summary = { teams: 0, sent: 0, skipped: null } try { - // Two cheap gates before any query that costs anything. Forums switched off + // Three cheap gates before any query that costs anything. Forums switched off // means the content this digest summarises is not readable on the site - // either, and un-configured email means there is no sink at all (§6.4). + // either; un-configured email means there is no sink at all (§6.4); and no + // enabled rule means the operator has not turned Team email on. if (!(await forumSettings.forumsEnabled())) return { ...summary, skipped: 'forums-disabled' } if (!(await mailer.isConfigured())) return { ...summary, skipped: 'email-unconfigured' } + const rule = await digestRule() + if (!rule) return { ...summary, skipped: 'no-enabled-rule' } const floor = new Date(now.getTime() - MAX_LOOKBACK_MS) const teams = await teamNotify.teamsWithForumActivitySince(floor) @@ -110,15 +196,27 @@ async function tick(now = new Date()) { for (const team of teams) { // eslint-disable-next-line no-await-in-loop const rows = await teamNotify.emailRecipients(team.id) - for (const r of rows.filter((x) => x.email_mode === 'digest')) { + const wanted = rows.filter((x) => x.email_mode === 'digest') + if (!wanted.length) continue + // The stamps now live in their own table, so they are read here rather than + // arriving on the recipient row. One query per Team, not one per recipient. + // eslint-disable-next-line no-await-in-loop + const stamps = await digestDb.stampsFor( + wanted.map((r) => r.user_id), + 'email', + notify.scopeKey(team), + ) + for (const r of wanted) { try { - // Serial, like the immediate sender and for the same reason: one SMTP + // Serial, like the instant path and for the same reason: one SMTP // conversation at a time against a provider with its own rate limits. // eslint-disable-next-line no-await-in-loop - if (await sendOne(team, r, now)) summary.sent += 1 + const recipient = { ...r, last_digest_at: stamps.get(Number(r.user_id)) ?? null } + // eslint-disable-next-line no-await-in-loop + if (await sendOne(team, recipient, rule, now)) summary.sent += 1 } catch (err) { // One recipient's failure must not end the sweep for the rest. The - // unstamped preference means the next run retries this one. + // unstamped window means the next run retries this one. log.warn('digest send failed', { teamId: team.id, userId: r.user_id, message: err.message }) } } @@ -155,4 +253,15 @@ function stop() { } } -module.exports = { start, stop, tick, clampSince, MAX_LOOKBACK_MS, MAX_ITEMS } +module.exports = { + start, + stop, + tick, + sendOne, + clampSince, + digestRule, + MAX_LOOKBACK_MS, + MAX_ITEMS, + DIGEST_TRIGGERS, + DEFAULT_TEMPLATE, +} diff --git a/server/src/utils/teamNotify.js b/server/src/utils/teamNotify.js index 0869014..8e47ea0 100644 --- a/server/src/utils/teamNotify.js +++ b/server/src/utils/teamNotify.js @@ -1,8 +1,21 @@ -// ── Team notification fan-out (TEAMS.md Part 6, phase 6) ─────────────────── +// ── Team notification fan-out (TEAMS.md Part 6; migrated in ENGAGEMENT.md Phase 6) ── // -// One event in, up to two sinks out: a content-free push tickle and — for forum -// content only — an email. The expensive part of a notification is working out -// who should get it, and that is computed once here and handed to both. +// One event in, three sinks out — and **as of Phase 6 only two of them are still +// this file's**. The expensive part of a notification is working out who should +// get it, and that is still computed once here and handed to all three. +// +// 1. the content-free push tickle — direct, here +// 2. the Discord bridge — direct, here +// 3. **email — now the engagement engine's**, reached by `events.emit` +// +// **Why email left and the other two did not.** A channel in the engine's sense +// is a per-recipient sink with a preference, an address and a digest mode; email +// is one, the bridge is not (its audience is whoever can read a Discord channel, +// which is why §3.1 warns against unifying a *leg* with a *channel*), and push's +// `deliver` belongs to Phase 7, which is when the inbox gives a tickle a `ref` +// worth deep-linking. Moving push a phase early would also have meant +// reconciling its per-Team opt-OUT with a registry whose `defaultMode` is `off`, +// and getting that wrong silences every existing member. // // **Nothing in this file ever throws.** Every entry point is called from a path // that has already done the real work: a forum reply is written and answered @@ -17,30 +30,28 @@ // is a destination the recipient chose rather than a relay (§6.4). The asymmetry // is the security model, not an inconsistency to tidy up. // -// **Phase 8 added a THIRD sink, and it is a second delivery rather than a second -// pipeline.** `utils/teamBridge.js` takes the same event, already computed, and -// hands it to a Discord channel the operator configured — which is why every -// entry point below calls it beside the tickle instead of anything re-deriving -// the event. Note that the bridge does NOT take the recipient set: its audience -// is whoever can read a channel, which is why enabling it for members-only -// content needs an operator acknowledgement (§7.2, teamIntegration.model.js). +// **The recipient set is computed HERE and travels on the envelope.** It is the +// same access-checked query it has always been (`teamNotify.recipientIds`, which +// asks the two tables `teamAccess.forumAccess()` asks), and the engine resolves a +// `members` audience to exactly it. That is Phase 6's decision 2, and the reason +// for it is that a saved audience segment composes module-declared lists with +// CONSTANT parameters — it cannot express "the members of the Team this post was +// in", because the answer is different for every firing. Core does not learn what +// a Team is; the event says who it is about. // -// **Roster events are push-only, and forum events are the only ones that email.** -// §6.4's argument for the email sink is the web-only user who never learns that -// someone replied to their own thread. "Someone joined the guild" is not that: it -// arrives from a sweep that runs every fifteen minutes, it is already on the -// activity feed, and mailing it is how a notification feature earns a spam -// complaint. The streams exist for all four events; the SINKS differ, and this is -// the file that says so. +// **Roster events now emit too, and they still do not mail anybody by default.** +// §6.4's argument for keeping "someone joined" out of the mail — it arrives from +// a fifteen-minute sweep, it is already on the activity feed, and mailing it is +// how a notification feature earns a spam complaint — is now expressed as a rule +// an operator has to enable rather than as a sink this file declines to call. The +// four rules core seeds are all disabled; the argument survives as the default. const pushDispatch = require('./pushDispatch') const teamBridge = require('./teamBridge') const teamNotify = require('../model/teams/teamNotify.model') const forumSettings = require('../model/teams/teamForumSettings.model') -const mailer = require('./mailer') +const engagementEmit = require('./engagementEmit') const registries = require('../modules/registries') -const unsubscribeToken = require('./unsubscribeToken') -const brand = require('../config/brand') const log = require('./logger')('team-notify') const STREAMS = { @@ -56,6 +67,9 @@ const EXCERPT_CHARS = 200 const baseUrl = () => (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') +/** The scope an unsubscribe link and a per-Team preference are keyed on. */ +const scopeKey = (team) => `team:${Number(team.id)}` + /** * Where this Team's page lives, or null. * @@ -65,39 +79,41 @@ const baseUrl = () => (process.env.APP_BASE_URL || 'http://localhost:5173').repl * gets email that names the Team and cannot link to it, which is a worse email * and not a broken one. */ -function teamPageUrl(team) { +function teamPagePath(team) { const provider = registries.registeredTeamProvider() const template = provider && provider.pageUrlTemplate if (!template || !team) return null - const path = template + return template .replace('{externalId}', encodeURIComponent(team.external_id ?? team.externalId ?? '')) .replace('{slug}', encodeURIComponent(team.slug ?? '')) - return `${baseUrl()}${path}` } -const threadUrl = (team, threadId) => { - const page = teamPageUrl(team) - // The forum navigates by SEARCH PARAM rather than by a route, because core has - // no route on a page it does not own (TeamForumPanel.jsx). So a deep link to a - // thread is the module's page plus `?thread=`, and it works under whatever path - // the module chose. +// **Two functions where there used to be one, and the split is the emit +// contract.** A trigger's `url` variables are validated site-RELATIVE +// (`engagementEmit.RELATIVE_URL`), because a variable that ends up in an href +// must not be able to carry an absolute one somewhere else; the mail renderer +// then absolutizes them against the deployment's base. The Discord bridge, which +// posts to a client that has no notion of this origin, still needs the absolute +// form. So the path is the value that travels and the URL is the value that is +// displayed. +const teamPageUrl = (team) => { + const path = teamPagePath(team) + return path ? `${baseUrl()}${path}` : null +} + +// The forum navigates by SEARCH PARAM rather than by a route, because core has +// no route on a page it does not own (TeamForumPanel.jsx). So a deep link to a +// thread is the module's page plus `?thread=`, and it works under whatever path +// the module chose. +const threadPath = (team, threadId) => { + const page = teamPagePath(team) return page ? `${page}?thread=${Number(threadId)}` : null } -// TWO urls from one token, and they are not interchangeable. -// -// `unsubscribeUrl` is the human one that goes in the mail body: the site's own -// page, which explains what is about to happen and POSTs once a person has read -// it. `unsubscribeApiUrl` is the machine one that goes in the `List-Unsubscribe` -// header, where RFC 8058 says a client may POST without showing anybody anything — -// so it has to be an endpoint, not a page. The API route answers GET on the same -// path with a redirect to the page, which covers the clients that render the -// header as an ordinary link. -const unsubscribeUrl = (userId, teamId) => - `${baseUrl()}/unsubscribe/${unsubscribeToken.sign(userId, teamId)}` - -const unsubscribeApiUrl = (userId, teamId) => - `${baseUrl()}/api/v1/public/teams/unsubscribe/${unsubscribeToken.sign(userId, teamId)}` +const threadUrl = (team, threadId) => { + const path = threadPath(team, threadId) + return path ? `${baseUrl()}${path}` : null +} const teamLabel = (team) => (team && (team.display_name_override || team.name)) || 'your team' @@ -123,20 +139,58 @@ async function tickle(streamId, team, { ref, exclude = [] } = {}) { return userIds.length } -// ── Roster events (push only, see the header) ────────────────────────────── - -// No `memberName` argument, and that is the point: a tickle is content-free, so -// there is nothing about WHO joined for this function to carry. The name is on -// the activity feed the app pulls after waking. -// -// `count` is phase 8's one addition and it is for the BRIDGE, not the tickle: a -// Discord channel has no app on the other end to pull anything, so the message -// has to say something, and "3 new members joined" is the most a caller that -// notifies once per sweep can honestly say. Optional, so the sync is the only -// caller that has to know it exists. -async function memberJoined(team, { count } = {}) { +/** + * Hand one Team event to the engagement engine. + * + * Fire-and-forget by construction: `emit` validates the payload and dispatches + * without awaiting (see engagementEmit's header), so this returns as soon as the + * contract has been checked. A refused emit is a contract bug — it throws in + * development and is logged in production — and either way it must not reach the + * caller, which is a forum write that has already replied. + * + * `recipientUserIds` is the SAME set the tickle used, so the two sinks cannot + * disagree about who this event is for. + */ +function emitTeamEvent(triggerId, team, data, recipientUserIds, { dedupeKey } = {}) { try { + if (!recipientUserIds.length) return false + const result = engagementEmit.emit('core', triggerId, { + data: { teamName: teamLabel(team), ...data }, + scopeKey: scopeKey(team), + recipientUserIds, + dedupeKey, + }) + return Boolean(result && result.ok) + } catch (err) { + log.warn('team event not emitted', { trigger: triggerId, teamId: team && team.id, message: err.message }) + return false + } +} + +// ── Roster events ────────────────────────────────────────────────────────── + +// No `memberName` argument on the TICKLE, and that is the point: a tickle is +// content-free, so there is nothing about WHO joined for it to carry. The name is +// on the activity feed the app pulls after waking. +// +// `count` is phase 8's addition and it is for the BRIDGE: a Discord channel has +// no app on the other end to pull anything, so the message has to say something, +// and "3 new members joined" is the most a caller that notifies once per sweep +// can honestly say. +// +// `names` is Phase 6's, and it is for the ENGINE. `team.member.joined` declares +// `memberName` required, so an event is emitted per joiner rather than per sweep: +// the trigger names one person, and there is no honest way to put five into a +// variable declared as one. What stops five joiners becoming five mails is the +// rule's own cooldown and its hourly ceiling — the mechanisms that exist for +// exactly this — rather than this file deciding on the operator's behalf. +async function memberJoined(team, { count, names = [] } = {}) { + try { + const recipientIds = await teamNotify.recipientIds(team.id) const sent = await tickle(STREAMS.MEMBER_JOINED, team, { ref: `team:${team.id}` }) + for (const memberName of names) { + emitTeamEvent(STREAMS.MEMBER_JOINED, team, { memberName, teamUrl: teamPagePath(team) }, recipientIds) + } await teamBridge.deliver(STREAMS.MEMBER_JOINED, team, { body: teamBridge.memberJoinedBody(count), teamUrl: teamPageUrl(team), @@ -149,9 +203,18 @@ async function memberJoined(team, { count } = {}) { } } -async function leadershipChanged(team) { +// `leaderName` is required by the declaration and means "the NEW leader", so an +// event is emitted per promotion and a run that only demoted somebody emits none. +// The tickle and the bridge still fire for either, exactly as before: "leadership +// changed" is a true thing to nudge about even when nobody was promoted, and it +// is not a true thing to name a new leader in. +async function leadershipChanged(team, { names = [] } = {}) { try { + const recipientIds = await teamNotify.recipientIds(team.id) const sent = await tickle(STREAMS.LEADERSHIP_CHANGED, team, { ref: `team:${team.id}` }) + for (const leaderName of names) { + emitTeamEvent(STREAMS.LEADERSHIP_CHANGED, team, { leaderName, teamUrl: teamPagePath(team) }, recipientIds) + } await teamBridge.deliver(STREAMS.LEADERSHIP_CHANGED, team, { body: 'Leadership has changed.', teamUrl: teamPageUrl(team), @@ -164,7 +227,7 @@ async function leadershipChanged(team) { } } -// ── Forum events (push + immediate email) ────────────────────────────────── +// ── Forum events ─────────────────────────────────────────────────────────── /** * A new thread or reply. @@ -173,9 +236,15 @@ async function leadershipChanged(team) { * the thing a leader wants everyone to read and mute the day-to-day chatter, * which is the split §6.2 drew and the reason there are four streams and not two. * - * The author is excluded from both sinks. Not as a nicety — a forum that emails - * you your own post is the first thing anyone turns off, and turning it off costs - * the deployment every other notification too. + * The author is excluded from the tickle and from the emitted audience. Not as a + * nicety — a forum that emails you your own post is the first thing anyone turns + * off, and turning it off costs the deployment every other notification too. + * + * @returns {{push: number, emitted: boolean, bridged: boolean}} `emitted` says the + * event reached the engine, NOT that anybody was mailed. Whether a mail goes out + * is a rule's answer and arrives asynchronously through the outbox; a caller + * that reported "3 emails sent" from here would be reporting a decision that has + * not been taken yet. The old `emails` count is gone for that reason. */ async function forumPost({ team, threadId, threadTitle, type, authorUserId, authorName, bodyHtml }) { try { @@ -183,12 +252,30 @@ async function forumPost({ team, threadId, threadTitle, type, authorUserId, auth // digest worker has no route in front of it, so the check has to live here as // well as there — and a switch flipped between a write and its notification // must silence the notification. - if (!(await forumSettings.forumsEnabled())) return { push: 0, emails: 0, bridged: false } + if (!(await forumSettings.forumsEnabled())) return { push: 0, emitted: false, bridged: false } - const stream = type === 'announcement' ? STREAMS.ANNOUNCEMENT : STREAMS.FORUM_POST + const announcement = type === 'announcement' + const stream = announcement ? STREAMS.ANNOUNCEMENT : STREAMS.FORUM_POST const exclude = authorUserId ? [authorUserId] : [] + const recipientIds = await teamNotify.recipientIds(team.id, { exclude }) const push = await tickle(stream, team, { ref: `team:${team.id}:thread:${threadId}`, exclude }) - const emails = await emailImmediate({ team, threadId, threadTitle, type, exclude, authorName, bodyHtml }) + + const emitted = emitTeamEvent( + stream, + team, + { + authorName: authorName || 'Someone', + // The two triggers name this differently and deliberately: an + // announcement has a `title`, a forum post belongs to a `threadTitle`. + ...(announcement ? { title: threadTitle } : { threadTitle }), + excerpt: excerpt(bodyHtml), + postUrl: threadPath(team, threadId), + }, + recipientIds, + // One post is one event however many times a retry re-runs this path. + { dedupeKey: `team:${team.id}:thread:${threadId}:${type || 'post'}` }, + ) + // The bridge is NOT given `exclude`. Excluding the author is a property of a // per-recipient sink — nobody wants their own post mailed back to them — and a // channel has no per-recipient anything. Suppressing the message because the @@ -199,52 +286,13 @@ async function forumPost({ team, threadId, threadTitle, type, authorUserId, auth url: threadUrl(team, threadId), teamUrl: teamPageUrl(team), }) - return { push, emails, bridged } + return { push, emitted, bridged } } catch (err) { log.warn('forum notification failed', { teamId: team && team.id, message: err.message }) - return { push: 0, emails: 0, bridged: false } + return { push: 0, emitted: false, bridged: false } } } -/** - * The `immediate` email mode: one mail per event, to the people who asked for - * exactly that. - * - * Skipped entirely when no email is configured — §6.4's "off unless configured" - * — and checked BEFORE the recipient query so a deployment with no mail - * transport configured pays nothing for the sink it does not have. - */ -async function emailImmediate({ team, threadId, threadTitle, type, exclude, authorName, bodyHtml }) { - if (!(await mailer.isConfigured())) return 0 - const rows = await teamNotify.emailRecipients(team.id, { exclude }) - const recipients = rows.filter((r) => r.email_mode === 'immediate') - if (recipients.length === 0) return 0 - - const label = teamLabel(team) - const kind = type === 'announcement' ? 'announcement' : 'post' - const url = threadUrl(team, threadId) - let sent = 0 - - for (const r of recipients) { - // Serial rather than Promise.all: this is an SMTP conversation per recipient - // against a relay with its own rate limits, and a burst of them from a - // busy thread is how a sending account gets throttled. The loop is also why the - // send below is fire-and-report rather than fire-and-throw. - // eslint-disable-next-line no-await-in-loop - const res = await mailer.sendTeamNotification({ - to: r.email, - subject: `[${brand.name}] ${label}: ${threadTitle}`, - intro: `${authorName || 'Someone'} posted a new ${kind} in ${label}.`, - items: [{ heading: threadTitle, excerpt: excerpt(bodyHtml), url }], - teamUrl: teamPageUrl(team), - unsubscribeUrl: unsubscribeUrl(r.user_id, team.id), - unsubscribeApiUrl: unsubscribeApiUrl(r.user_id, team.id), - }) - if (res && res.sent) sent += 1 - } - return sent -} - module.exports = { STREAMS, memberJoined, @@ -253,10 +301,12 @@ module.exports = { // Exported for the digest worker and for the tests, which is the whole reason // they are not inlined: a URL that only ever appears inside a mail body is a // URL nothing can assert on. + teamPagePath, teamPageUrl, + threadPath, threadUrl, - unsubscribeUrl, - unsubscribeApiUrl, + scopeKey, excerpt, teamLabel, + emitTeamEvent, } diff --git a/server/src/utils/unsubscribeToken.js b/server/src/utils/unsubscribeToken.js index fb4dc8c..54b2328 100644 --- a/server/src/utils/unsubscribeToken.js +++ b/server/src/utils/unsubscribeToken.js @@ -1,6 +1,6 @@ -// ── One-click unsubscribe tokens (TEAMS.md §6.4) ─────────────────────────── +// ── One-click unsubscribe tokens (TEAMS.md §6.4; generalized in ENGAGEMENT.md Phase 6) ── // -// A stateless HMAC over (userId, teamId, version), not a row in a table. +// A stateless HMAC over the thing being unsubscribed from, not a row in a table. // // **Why stateless.** The alternative is a `password_resets`-shaped token table, // and it is the wrong shape for this: an unsubscribe link sits in a mailbox for @@ -9,17 +9,32 @@ // table would need pruning for a capability that never expires. Every property // that makes a reset token a row is absent here. // -// **What the capability actually is.** Holding a token lets the holder set -// `muted = 1` for ONE (user, Team) pair. It cannot read anything, cannot unmute, -// cannot touch email mode, and names no other Team. So the honest threat model is: -// someone who intercepts the mail can silence one Team's notifications for that -// account, visibly and reversibly on the account screen. That is a smaller -// capability than the mail itself already carries (it contains the content). +// **v1 was `(userId, teamId)`; v2 is `(userId, channel, scopeKey)`, and BOTH +// verify — permanently.** Phase 6 generalized the token because the thing being +// unsubscribed from is no longer always a Team, but v1 tokens are already in +// people's mailboxes and a link that stops working is a person who cannot +// unsubscribe. A v1 token reads as `{ channel: 'email', scopeKey: 'team:' }`: +// it can only ever have arrived in an email, so naming that channel is a reading +// of what it always meant rather than a guess. +// +// **What the capability actually is.** Holding a token lets the holder turn ONE +// channel off for ONE scope for one account. It cannot read anything, cannot turn +// anything back on, and names no other scope. So the honest threat model is: +// someone who intercepts the mail can silence one Team's email for that account, +// visibly and reversibly on the account screen. That is a smaller capability than +// the mail itself already carries (it contains the content). +// +// **The narrowing from v1 is deliberate and is a live behaviour change.** A v1 +// token set `muted = 1`, which silenced that Team's push as well as its email — +// a link labelled "stop these emails" quietly stopped notifications on somebody's +// phone. From this phase a token turns off the channel it names and nothing else, +// which is both what the link says and what RFC 8058 means by it. Settled by the +// org lead 2026-08-29. // // **`v` is the version prefix, and it is what makes rotation possible at all.** A -// stateless token cannot be revoked individually; bumping VERSION invalidates -// every outstanding link at once, which is the only revocation a design with no -// server-side state can offer, and it needs to exist before it is needed. +// stateless token cannot be revoked individually; retiring a version invalidates +// every outstanding link of it at once, which is the only revocation a design +// with no server-side state can offer, and it needs to exist before it is needed. // // The key is SECRET_ENC_KEY, derived through the same dev fallback as // utils/secretBox — a separate label so an unsubscribe token can never be @@ -30,7 +45,20 @@ require('dotenv').config() const log = require('./logger')('unsub-token') -const VERSION = 1 +// The version this deployment SIGNS with. Both are verified; see the header. +const VERSION = 2 +const LEGACY_VERSION = 1 + +// `.` is the field separator, so neither field may contain one. The scope +// vocabulary is `:` (`team:12`) or '' for deployment-wide, and the +// channel ids the registry accepts are `[a-z][a-z0-9_.-]*` — which DOES admit a +// dot (`discord.dm` is the example §3.1 gives). So the channel is checked against +// a dot-free subset here rather than against the registry's own pattern, and a +// channel id containing a dot would need a signing format with a real escape +// before it could carry an unsubscribe link. Refused loudly rather than signed +// into a token that verifies as some other channel. +const CHANNEL_RE = /^[a-z][a-z0-9_-]*$/ +const SCOPE_RE = /^[a-z0-9][a-z0-9:_-]*$/ function resolveKey() { const explicit = process.env.SECRET_ENC_KEY @@ -57,35 +85,79 @@ const key = () => { // client's own re-wrapping of a long URL without any of the three escaping it. const b64u = (buf) => buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') -function sign(userId, teamId) { - const body = `${VERSION}.${Number(userId)}.${Number(teamId)}` - const mac = crypto.createHmac('sha256', key()).update(body).digest() - // Truncated to 16 bytes (128 bits). Full-length would double the URL for no - // reachable gain: forging this buys one mute, and 128 bits is far past the - // point where that is worth anyone's compute. - return `${body}.${b64u(mac.subarray(0, 16))}` -} +// Truncated to 16 bytes (128 bits). Full-length would double the URL for no +// reachable gain: forging this buys one unsubscribe, and 128 bits is far past the +// point where that is worth anyone's compute. +const mac = (body) => b64u(crypto.createHmac('sha256', key()).update(body).digest().subarray(0, 16)) + +const legacyBody = (userId, teamId) => `${LEGACY_VERSION}.${Number(userId)}.${Number(teamId)}` /** - * Verify a token. Returns { userId, teamId } or null — null for every failure - * mode, deliberately, so a caller cannot accidentally report which part was wrong. + * Sign a v2 token: turn `channel` off for `scopeKey` for this user. + * + * @param {number} userId + * @param {string} channel a registered delivery-channel id, dot-free (see CHANNEL_RE) + * @param {string} scopeKey '' for deployment-wide, or `:` — a stable + * IDENTIFIER, never a display name. A Team renamed + * between the mail and the click must not orphan the + * link in it, which is why this is not `subject_key`. */ -function verify(token) { - const parts = String(token || '').split('.') - if (parts.length !== 4) return null - const [v, uid, tid] = parts - if (Number(v) !== VERSION) return null - const userId = Number(uid) - const teamId = Number(tid) - if (!Number.isInteger(userId) || !Number.isInteger(teamId)) return null - - const expected = sign(userId, teamId) - const a = Buffer.from(expected) - const b = Buffer.from(String(token)) - // Length-check first: timingSafeEqual throws on a length mismatch, and the - // length of a token is not a secret. - if (a.length !== b.length || !crypto.timingSafeEqual(a, b)) return null - return { userId, teamId } +function sign(userId, channel, scopeKey = '') { + const id = Number(userId) + if (!Number.isInteger(id) || id < 1) throw new Error('unsubscribeToken.sign: userId must be a positive integer') + if (!CHANNEL_RE.test(String(channel || ''))) { + throw new Error(`unsubscribeToken.sign: channel "${channel}" cannot be carried in a token`) + } + const scope = String(scopeKey || '') + if (scope && !SCOPE_RE.test(scope)) { + throw new Error(`unsubscribeToken.sign: scope "${scope}" cannot be carried in a token`) + } + const body = `${VERSION}.${id}.${channel}.${scope}` + return `${body}.${mac(body)}` } -module.exports = { sign, verify, VERSION } +/** Sign a v1 token. Kept only so a test can produce one; nothing else calls it. */ +const signLegacy = (userId, teamId) => `${legacyBody(userId, teamId)}.${mac(legacyBody(userId, teamId))}` + +/** + * Verify a token of either version. + * + * Returns `{ userId, channel, scopeKey, version }` or null — null for every + * failure mode, deliberately, so a caller cannot accidentally report which part + * was wrong. + */ +function verify(token) { + const raw = String(token || '') + const parts = raw.split('.') + if (parts.length < 4) return null + + if (Number(parts[0]) === LEGACY_VERSION) { + if (parts.length !== 4) return null + const userId = Number(parts[1]) + const teamId = Number(parts[2]) + if (!Number.isInteger(userId) || !Number.isInteger(teamId)) return null + if (!equal(signLegacy(userId, teamId), raw)) return null + // A v1 link can only ever have arrived in an email. See the header. + return { userId, channel: 'email', scopeKey: `team:${teamId}`, version: LEGACY_VERSION } + } + + if (Number(parts[0]) !== VERSION || parts.length !== 5) return null + const userId = Number(parts[1]) + const channel = parts[2] + const scopeKey = parts[3] + if (!Number.isInteger(userId) || userId < 1) return null + if (!CHANNEL_RE.test(channel)) return null + if (scopeKey && !SCOPE_RE.test(scopeKey)) return null + if (!equal(sign(userId, channel, scopeKey), raw)) return null + return { userId, channel, scopeKey, version: VERSION } +} + +function equal(expected, actual) { + const a = Buffer.from(expected) + const b = Buffer.from(actual) + // Length-check first: timingSafeEqual throws on a length mismatch, and the + // length of a token is not a secret. + return a.length === b.length && crypto.timingSafeEqual(a, b) +} + +module.exports = { sign, signLegacy, verify, VERSION, LEGACY_VERSION, CHANNEL_RE, SCOPE_RE } diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 480ea63..5d68a0f 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -13424,6 +13424,67 @@ } } }, + "/api/v1/public/engagement/unsubscribe/{token}": { + "post": { + "tags": [ + "Public · Engagement" + ], + "summary": "Unsubscribe from one channel for one scope", + "description": "Honours the tokened link in an engagement email, including RFC 8058 one-click. The token names a delivery channel and a scope; the write turns that channel off for that scope and nothing else. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, scope) pairs exist. Tokens signed before this route existed are still honoured, at this path and at the older /public/teams one.", + "parameters": [ + { + "name": "token", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "The signed token from the email link." + } + ], + "responses": { + "200": { + "description": "Acknowledged", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OkFlag" + } + } + } + } + }, + "security": [ + {} + ] + }, + "get": { + "tags": [ + "Public · Engagement" + ], + "summary": "Land a human on the unsubscribe page", + "description": "For mail clients that render the List-Unsubscribe URL as an ordinary link. Redirects to the site’s own confirmation page and changes nothing — a GET must not mutate, or a link scanner would unsubscribe people who asked for nothing.", + "parameters": [ + { + "name": "token", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "The signed token from the email link." + } + ], + "responses": { + "302": { + "description": "Redirect to the site’s unsubscribe page" + } + }, + "security": [ + {} + ] + } + }, "/api/v1/public/modules": { "get": { "tags": [ @@ -13811,8 +13872,8 @@ "tags": [ "Public · Teams" ], - "summary": "Unsubscribe from one Team’s notification emails", - "description": "Honours the tokened link in a Team notification email, including RFC 8058 one-click. Sets the same per-Team mute the account screen shows. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.", + "summary": "Unsubscribe from one Team’s notification emails (legacy path)", + "description": "The pre-Phase-6 path, kept permanently because links in mail already sent point at it. Identical to POST /public/engagement/unsubscribe/{token}. Honours the tokened link including RFC 8058 one-click; a token signed before Phase 6 turns off that Team’s email and no longer mutes its push. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.", "parameters": [ { "name": "token", diff --git a/server/test/emailTemplates.test.js b/server/test/emailTemplates.test.js index 69503fa..9ef94af 100644 --- a/server/test/emailTemplates.test.js +++ b/server/test/emailTemplates.test.js @@ -182,6 +182,29 @@ test('a variable carrying a javascript: url never becomes an href', () => { assert.match(out.html, /Press me/) // inert, but not silently vanished }) +// **A defect until Phase 6, and the phase that put a rule-driven variable in a +// button is the one that could see it.** `email.image` and `email.itemList` both +// absolutize; `email.button` did not. A trigger's `url` variables are validated +// site-RELATIVE by construction (`engagementEmit.RELATIVE_URL`), so every +// rule-driven CTA interpolated to `/guilds/x` — a path a mail client has no +// origin to resolve, i.e. a dead link in every notification the engine sends. +test('a relative url in a button is absolutized, in both parts', () => { + const ctx = emailBlocks.buildContext({ values: { link: '/guilds/silver-anvil?thread=7' }, baseUrl: BASE }) + const block = { id: 'b', type: 'email.button', props: { label: 'Read it', url: '{{link}}' } } + const out = emailBlocks.renderBlocks([block], ctx) + assert.equal(out.html.includes(`href="${BASE}/guilds/silver-anvil?thread=7"`), true) + assert.equal(out.text.includes(`${BASE}/guilds/silver-anvil?thread=7`), true) +}) + +test('an absolute url in a button is left exactly as it is', () => { + const ctx = emailBlocks.buildContext({ values: { link: 'https://elsewhere.test/x' }, baseUrl: BASE }) + const out = emailBlocks.renderBlocks( + [{ id: 'b', type: 'email.button', props: { label: 'Go', url: '{{link}}' } }], + ctx, + ) + assert.match(out.html, /href="https:\/\/elsewhere\.test\/x"/) +}) + test('a literal unsafe url is refused at save, and a tokened one is allowed through', () => { const bad = emailBlocks.validateEmailBlocks([ { id: 'b', type: 'email.button', props: { label: 'x', url: 'javascript:alert(1)' } }, diff --git a/server/test/engagementEmail.test.js b/server/test/engagementEmail.test.js new file mode 100644 index 0000000..edbaa35 --- /dev/null +++ b/server/test/engagementEmail.test.js @@ -0,0 +1,408 @@ +// ── The email channel on the engine (ENGAGEMENT.md Phase 6) ──────────────── +// +// The phase's own acceptance criteria, plus the four things building it showed +// were worth pinning: +// +// • a scoped preference is what decides a Team-scoped event, and it REPLACES +// the stream-level one — intersecting would silence every existing subscriber +// • the structural projection fills only what the payload did not +// • a `members` audience resolves to the set the EVENT carried +// • core's four seeded rules exist, are all disabled, and are seeded once +// +// 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 scopedPrefs = require('../src/engagement/scopedPrefs') +const projection = require('../src/engagement/projection') +const audiences = require('../src/engagement/audiences') +const engine = require('../src/engagement/engine') +const emailChannel = require('../src/engagement/emailChannel') +const coreRules = require('../src/engagement/coreRules') +const templates = require('../src/engagement/templates') +const mailer = require('../src/utils/mailer') +const unsubscribeToken = require('../src/utils/unsubscribeToken') +const engagementEmit = require('../src/utils/engagementEmit') +const rulesDb = require('../src/model/engagement/engagementRules.db') +const recipients = require('../src/model/engagement/engagementRecipients.db') +const settingsDb = require('../src/model/settings/settings.db') +const teamNotifyModel = require('../src/model/teams/teamNotify.model') +const unsubCtrl = require('../src/router/v1/public/engagement.controller') +const db = require('../src/utils/db') + +// Requiring this is what registers core's channels, transports and the `team` +// scope provider — the one door (engagement/index.js's header). It does NOT +// register core's TRIGGERS: those come from `registries.registerCore()`, which +// app.js calls, and the split is deliberate — a trigger is a module-facing +// declaration and a channel is an internal sink. +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: [], teamPrefs: new Map(), storedModes: new Map(), inserted: [], settings: new Map() } + + patch(mailer, 'sendNotification', async (msg) => { + world.mails.push(msg) + return { ok: true, transport: 'smtp' } + }) + patch(templates, 'renderByKey', async (key, values) => ({ + subject: `[${key}] ${values.title || ''}`, + html: '

x

', + text: 'x', + missing: [], + values, + })) + patch(recipients, 'addressFor', async (userId) => ({ address: `u${userId}@example.test` })) + patch(recipients, 'filterActive', async (ids) => ids) + patch(recipients, 'storedModes', async () => world.storedModes) + patch(teamNotifyModel, 'prefsForTeam', async (userIds, teamId) => + userIds + .map((id) => world.teamPrefs.get(`${id}|${teamId}`)) + .filter(Boolean)) + patch(rulesDb, 'getById', async () => ({ + id: 1, + trigger_id: TRIGGER, + template_keys: { email: 'notify.team-post' }, + })) +}) +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: { teamName: 'The Silver Hand', authorName: 'Ten', threadTitle: 'Raid', postUrl: '/g/1?thread=7' }, + ...over, +}) + +// ── deliver ──────────────────────────────────────────────────────────────── + +test('a delivered row renders its rule’s template and sends to the user’s address', async () => { + const result = await emailChannel.deliver(outboxRow()) + assert.equal(result.ok, true) + assert.equal(world.mails[0].to, 'u11@example.test') + assert.match(world.mails[0].rendered.subject, /^\[notify\.team-post\]/) +}) + +test('a rule naming no template falls back to the generic one, which is what makes a new trigger mailable', async () => { + patch(rulesDb, 'getById', async () => ({ id: 1, trigger_id: TRIGGER, template_keys: {} })) + await emailChannel.deliver(outboxRow()) + assert.match(world.mails[0].rendered.subject, /^\[notify\.event\]/) +}) + +// Terminal, not retryable. Retrying does not give somebody an address, and a +// banned account will not be un-banned by a five-minute backoff. +test('a user with no deliverable address is a terminal failure, not a retry', async () => { + patch(recipients, 'addressFor', async () => null) + const result = await emailChannel.deliver(outboxRow()) + assert.equal(result.ok, false) + assert.equal(result.retry, undefined) + assert.equal(world.mails.length, 0) +}) + +// A throw would be read by the worker as a transient failure and retried five +// times, so an unrenderable template would become five identical rows in the send +// log instead of one honest terminal one. +test('deliver never throws — a render failure is classified, not propagated', async () => { + patch(templates, 'renderByKey', async () => { throw new Error('blocks are broken') }) + const result = await emailChannel.deliver(outboxRow()) + assert.equal(result.ok, false) + assert.match(result.detail, /blocks are broken/) +}) + +test('the send log gets a hash of the address and never the address', async () => { + const result = await emailChannel.deliver(outboxRow()) + assert.match(result.addressHash, /^[0-9a-f]{64}$/) + assert.equal(result.addressHash.includes('@'), false) +}) + +// A bounce arrives with an address, not with a spelling. Two spellings of one +// mailbox must hash to one row or the correlation Phase 9 needs cannot be made. +test('the address hash is case-folded, so a bounce can be correlated', async () => { + const a = await emailChannel.deliver(outboxRow()) + patch(recipients, 'addressFor', async () => ({ address: ' U11@Example.Test ' })) + const b = await emailChannel.deliver(outboxRow()) + assert.equal(a.addressHash, b.addressHash) +}) + +// ── The unsubscribe link ─────────────────────────────────────────────────── + +test('every mail carries both unsubscribe urls, and they are not the same url', async () => { + await emailChannel.deliver(outboxRow()) + const { unsubscribeUrl, unsubscribeApiUrl } = world.mails[0] + assert.notEqual(unsubscribeUrl, unsubscribeApiUrl) + // The header one has to be an ENDPOINT — a one-click client POSTs to it without + // rendering anything — and the body one has to be a page a human can read first. + assert.match(unsubscribeApiUrl, /\/api\/v1\/public\/engagement\/unsubscribe\//) + assert.match(unsubscribeUrl, /\/unsubscribe\//) +}) + +test('the link’s token names the email channel and the event’s SCOPE, not its subject', async () => { + await emailChannel.deliver(outboxRow()) + const token = world.mails[0].unsubscribeApiUrl.split('/').pop() + assert.deepEqual(unsubscribeToken.verify(token), { + userId: 11, channel: 'email', scopeKey: 'team:1', version: 2, + }) + // `subject_key` is the Team's NAME, which a rename changes. Signing over it + // would orphan every link in a mailbox the first time staff renamed a guild. + assert.equal(token.includes('Silver'), false) +}) + +test('a scope the token format cannot carry costs the link, not the mail', async () => { + const result = await emailChannel.deliver(outboxRow({ scope_key: 'NOT A SCOPE' })) + assert.equal(result.ok, true) + assert.equal(world.mails[0].unsubscribeUrl, null) +}) + +// ── The projection (§4.6.1 property 1) ───────────────────────────────────── + +test('the projection fills only what the payload left out', () => { + const declaration = registries.eventTrigger(TRIGGER) + assert.ok(declaration, 'core registers the four Team triggers') + + const values = projection.project(TRIGGER, { teamName: 'X', threadTitle: 'Raid', postUrl: '/g/1' }) + assert.equal(values.threadTitle, 'Raid') // untouched + assert.equal(values.title, declaration.label) // supplied + assert.equal(values.intro, declaration.description) + assert.equal(values.actionUrl, '/g/1') // the first declared url with a value +}) + +// `news.post` and `team.announcement` both declare their own `title`. A +// projection that overwrote it would replace a real headline with a category +// label — on the one variable every generic template puts in the subject line. +test('a payload that declares its own title keeps it', () => { + const values = projection.project('team.announcement', { teamName: 'X', title: 'Siege moved' }) + assert.equal(values.title, 'Siege moved') +}) + +test('an unregistered trigger still renders from its snapshot rather than being refused', () => { + const values = projection.project('gone.away', { title: 'kept' }) + assert.equal(values.title, 'kept') + assert.deepEqual(values.items, []) +}) + +// ── The event-carried audience (decision 2) ──────────────────────────────── + +test('a members audience resolves to the recipient set the event carried', async () => { + const resolved = await audiences.resolveForRule( + { audience: 'members', audience_segment_id: null }, + { triggerId: TRIGGER, recipientUserIds: [11, 12] }, + ) + assert.deepEqual(resolved.userIds, [11, 12]) + assert.equal(resolved.ceiling, 'members') + assert.equal(resolved.dormant, false) +}) + +test('a members audience with no carried set and no segment still reaches nobody', async () => { + const resolved = await audiences.resolveForRule( + { audience: 'members', audience_segment_id: null }, + { triggerId: TRIGGER }, + ) + assert.deepEqual(resolved.userIds, []) + assert.match(resolved.reason, /needs a segment/) +}) + +// The carried set is a NARROWING input. It names who the event is about; it does +// not raise what a rule is allowed to reach. +test('a carried audience is still filtered for account status', async () => { + patch(recipients, 'filterActive', async (ids) => ids.filter((id) => id !== 12)) + const resolved = await audiences.resolveForRule( + { audience: 'members', audience_segment_id: null }, + { triggerId: TRIGGER, recipientUserIds: [11, 12] }, + ) + assert.deepEqual(resolved.userIds, [11]) +}) + +test('and it is still under the trigger’s G24 ceiling', () => { + // `members` is what the four Team triggers ceiling at, so a carried set can + // never be given `authenticated` by a rule that names one. + assert.equal(audiences.permitted(TRIGGER, 'members'), true) + assert.equal(audiences.permitted(TRIGGER, 'authenticated'), false) +}) + +test('the emit contract refuses an audience that is not a list of user ids', () => { + const bad = (recipientUserIds) => + assert.throws(() => engagementEmit.emit('core', TRIGGER, { + data: { teamName: 'X', authorName: 'A', threadTitle: 'T' }, + recipientUserIds, + })) + bad('11') + bad([0]) + bad([1.5]) + bad(new Array(6000).fill(1).map((_, i) => i + 1)) +}) + +// ── Scoped preferences (decision 4) ──────────────────────────────────────── + +const teamPref = (userId, teamId, over) => world.teamPrefs.set(`${userId}|${teamId}`, { + user_id: userId, muted: 0, email_mode: 'off', ...over, +}) + +test('a Team-scoped email event is decided by team_notification_prefs', async () => { + teamPref(11, 1, { email_mode: 'immediate' }) + teamPref(12, 1, { email_mode: 'off' }) + const eligible = await engine.subscribedTo([11, 12], TRIGGER, 'email', 'team:1') + assert.deepEqual(eligible, [11]) +}) + +// **The heart of decision 4.** `notification_channel_prefs` holds a row only +// where a user expressed something, absence means the channel default, and +// email's is `off`. Nobody has ever expressed a stream-level opinion about a Team +// trigger — the screen that would let them is Phase 3's and the preference +// predates it. So intersecting the two would resolve every existing Team-email +// subscriber to `off` and silence the live pipeline on the migrating deploy. +test('the scoped preference REPLACES the stream one — it does not intersect with it', async () => { + teamPref(11, 1, { email_mode: 'immediate' }) + world.storedModes = new Map() // no stream-level row: the state of every real user + assert.equal(channels.defaultMode('email'), 'off') + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'email', 'team:1'), [11]) +}) + +test('a per-Team mute silences every channel, not only the one that carries content', async () => { + teamPref(11, 1, { muted: 1, email_mode: 'immediate' }) + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'email', 'team:1'), []) + world.storedModes = new Map([[11, 'instant']]) + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'push', 'team:1'), []) +}) + +test('a scope says nothing about push, so the stream preference decides it', async () => { + teamPref(11, 1, { email_mode: 'off' }) + world.storedModes = new Map([[11, 'instant']]) + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'push', 'team:1'), [11]) +}) + +test('an unscoped event is decided by the stream preference alone', async () => { + teamPref(11, 1, { email_mode: 'immediate' }) + world.storedModes = new Map() + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'email', null), []) +}) + +// Fails OPEN, and the practical effect is that nothing is sent rather than that +// everybody is: the stream-level default is `off`. Failing closed would instead +// drop an unrelated IDOC warning because a Team preference query timed out. +test('a scope provider that throws leaves the stream preference in charge', async () => { + patch(teamNotifyModel, 'prefsForTeam', async () => { throw new Error('db is on fire') }) + world.storedModes = new Map([[11, 'instant']]) + assert.deepEqual(await engine.subscribedTo([11], TRIGGER, 'email', 'team:1'), [11]) +}) + +test('an unparseable or unclaimed scope is "no scope", never some other scope', async () => { + assert.equal(scopedPrefs.parse('team:1').prefix, 'team') + assert.equal(scopedPrefs.parse('team:'), null) + assert.equal(scopedPrefs.parse(':1'), null) + assert.equal(scopedPrefs.parse(''), null) + assert.equal((await scopedPrefs.resolve([11], 'email', 'nosuch:1')).size, 0) +}) + +// ── The one-click unsubscribe (decision 7) ───────────────────────────────── + +test('a v2 email token turns off that Team’s email and leaves its push alone', async () => { + const writes = [] + patch(teamNotifyModel, 'setEmailMode', async (...a) => writes.push(['email', ...a])) + patch(teamNotifyModel, 'mute', async (...a) => writes.push(['mute', ...a])) + await unsubCtrl.applyClaim({ userId: 11, channel: 'email', scopeKey: 'team:1' }) + assert.deepEqual(writes, [['email', 11, 1, 'off']]) +}) + +// The acceptance criterion: a link from a mail sent BEFORE the migration still +// works. It arrives at the old path, verifies as a v1 token, and turns off the +// email it was labelled as turning off. +test('a pre-migration link still unsubscribes, through the same handler', async () => { + const writes = [] + patch(teamNotifyModel, 'setEmailMode', async (...a) => writes.push(a)) + const legacy = unsubscribeToken.signLegacy(11, 1) + const res = { json: (body) => { res.body = body } } + await unsubCtrl.unsubscribe({ params: { token: legacy } }, res) + assert.deepEqual(res.body, { ok: true }) + assert.deepEqual(writes, [[11, 1, 'off']]) +}) + +// An oracle for which (user, scope) pairs exist would be a real disclosure on an +// endpoint with no session behind it. +test('a forged token is answered exactly like a real one', async () => { + const writes = [] + patch(teamNotifyModel, 'setEmailMode', async (...a) => writes.push(a)) + const res = { json: (body) => { res.body = body } } + await unsubCtrl.unsubscribe({ params: { token: '2.11.email.team:1.AAAAAAAAAAAAAAAAAAAAAA' } }, res) + assert.deepEqual(res.body, { ok: true }) + assert.deepEqual(writes, []) +}) + +test('a GET on the unsubscribe endpoint mutates nothing and lands on the page', () => { + const writes = [] + patch(teamNotifyModel, 'setEmailMode', async (...a) => writes.push(a)) + let redirected = null + unsubCtrl.unsubscribeLanding( + { params: { token: unsubscribeToken.sign(11, 'email', 'team:1') } }, + { redirect: (code, url) => { redirected = { code, url } } }, + ) + assert.equal(redirected.code, 302) + assert.match(redirected.url, /\/unsubscribe\//) + assert.deepEqual(writes, [], 'a link scanner must not be able to unsubscribe anybody') +}) + +// ── The seeded rules (decision 3) ────────────────────────────────────────── + +test('core seeds a rule for each Team trigger, and every one of them is OFF', async () => { + patch(settingsDb, 'get', async () => null) + patch(settingsDb, 'set', async (k, v) => world.settings.set(k, v)) + patch(rulesDb, 'insert', async (rule) => { world.inserted.push(rule); return world.inserted.length }) + + const summary = await coreRules.seedTeamRules() + assert.equal(summary.inserted, 4) + assert.deepEqual( + world.inserted.map((r) => r.trigger_id).sort(), + ['team.announcement', 'team.forum.post', 'team.leadership.changed', 'team.member.joined'], + ) + // The invariant the org lead chose to honour rather than carve an exception + // into: nothing is seeded on. Team email resumes when an operator switches one + // on, and the release note says so. + assert.equal(world.inserted.every((r) => r.enabled === 0), true) + // `members`, which is what resolves to the carried recipient set. Anything + // wider would be refused by the trigger's own ceiling anyway. + assert.equal(world.inserted.every((r) => r.audience === 'members'), true) + assert.equal(world.inserted.every((r) => r.channels.includes('email')), true) +}) + +test('every seeded rule names a template that actually exists', () => { + const seeded = new Set(coreRules.RULES.flatMap((r) => Object.values(r.template_keys))) + const shipped = new Set(require('../src/engagement/templateSeeds').SEEDS.map((s) => s.key)) + for (const key of seeded) assert.equal(shipped.has(key), true, `${key} is not a shipped template`) +}) + +// Seeded once, not ensured: an operator who deletes a rule must not find it back +// after a restart, and one they enabled must not be reset to off. +test('a second boot seeds nothing', async () => { + patch(settingsDb, 'get', async () => '2026-08-29T00:00:00.000Z') + patch(rulesDb, 'insert', async () => { throw new Error('must not insert') }) + const summary = await coreRules.seedTeamRules() + assert.equal(summary.inserted, 0) + assert.equal(summary.skipped, 4) +}) diff --git a/server/test/engagementEngine.test.js b/server/test/engagementEngine.test.js index 612e792..2dbabe8 100644 --- a/server/test/engagementEngine.test.js +++ b/server/test/engagementEngine.test.js @@ -475,10 +475,11 @@ test('two sweepers racing one due row: exactly one claim wins', async () => { // ── The send log ─────────────────────────────────────────────────────────── test('a row whose channel has no deliver() finishes failed, and the send log says why', async () => { - // Phase 4a delivers nothing: `deliver` arrives with email in Phase 6 and the - // inbox in Phase 7. Recording 'sent' would be a lie in the one table whose - // purpose is answering "did they get it". - addRule() + // `inapp`, because as of Phase 6 `email` DOES deliver. The inbox arrives in + // Phase 7, and until then recording 'sent' would be a lie in the one table + // whose purpose is answering "did they get it". + addRule({ channels: ['inapp'] }) + optIn(10, 'uo.house.idoc_warning', 'inapp') await engine.dispatch(event(), T0) await worker.tick(later(1000)) @@ -545,14 +546,20 @@ test('absence means the CHANNEL default, and all three of core default off', asy assert.equal(channels.defaultMode('email'), 'off') }) -test("a 'digest' preference still enqueues — batching is the drain's job, not the enqueue's", async () => { +// **This reverses what Phase 4a asserted here**, and the reversal is Phase 6's +// §4.2b decision rather than a change of mind about queues. A digest is +// re-derived from the source tables at send time — that is what makes a hidden +// post absent from it and a user who lost access unreachable by it — so an outbox +// row for a digest recipient would be a second copy of the content with none of +// those properties. Nothing drains it, so nothing writes it. +test("a 'digest' preference does NOT enqueue — the digest re-derives at send time", async () => { registries._reset() registerUoTrigger({ ceiling: 'authenticated', audience: 'authenticated' }) optIn(10, 'uo.house.idoc_warning', 'email', 'digest') addRule({ audience: 'authenticated' }) await engine.dispatch(event(), T0) - assert.equal(outboxRows().length, 1) + assert.equal(outboxRows().length, 0) }) // ── The hourly ceiling (§7.1 Q3) ─────────────────────────────────────────── diff --git a/server/test/mailer.test.js b/server/test/mailer.test.js index 01d9119..d5a7c16 100644 --- a/server/test/mailer.test.js +++ b/server/test/mailer.test.js @@ -37,6 +37,11 @@ const configured = (over = {}) => ({ let sent let transportCfg +// An already-rendered body, which is what `sendNotification` takes: the email +// channel renders the template and this file's job is only the transport and the +// headers (ENGAGEMENT.md Phase 6). +const RENDERED = { subject: 'A subject', html: '

body

', text: 'body' } + beforeEach(() => { sent = null transportCfg = null @@ -78,10 +83,15 @@ test('unconfigured → password reset returns NOT_CONFIGURED (caller still answe assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) }) -test('unconfigured → team notification returns NOT_CONFIGURED and never throws', async () => { +// Unconfigured is RETRYABLE for this one sender, and it is the only one where +// that is the right answer: an operator halfway through typing SMTP credentials +// should find the outbox drains once they finish, not a backlog of rows the +// worker gave up on five minutes in. +test('unconfigured → an engagement send is a retryable failure, never a throw', async () => { emailConfig.getWithSecret = async () => null - const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) - assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) + const r = await mailer.sendNotification({ to: 'a@b.com', rendered: RENDERED }) + assert.equal(r.ok, false) + assert.equal(r.retry, true) }) test('unconfigured → only sendTest throws, because only sendTest has an admin waiting', async () => { @@ -145,8 +155,8 @@ test('an incomplete credential is unconfigured, not a crash', async () => { test('a stored transport id that is not registered degrades, it does not throw', async () => { emailConfig.getWithSecret = async () => configured({ transport: 'mailgun' }) - const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) - assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) + const r = await mailer.sendNotification({ to: 'a@b.com', rendered: RENDERED }) + assert.equal(r.ok, false) }) // ── failures ──────────────────────────────────────────────────────────────── @@ -180,11 +190,58 @@ test('a rejected sender is diagnosed by name — the failure mode SMTP introduce assert.match(recorded.statusDetail, /SPF\/DMARC/) }) -test('a team notification failure is swallowed, never thrown', async () => { +test('an engagement send failure is swallowed and classified, never thrown', async () => { emailConfig.recordStatus = async () => {} nodemailer.createTransport = () => ({ sendMail: async () => { throw new Error('relay down') } }) emailConfig.getWithSecret = async () => configured() - const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) - assert.deepEqual(r, { sent: false, reason: 'SEND_FAILED' }) + const r = await mailer.sendNotification({ to: 'a@b.com', rendered: RENDERED }) + assert.equal(r.ok, false) + // Transient: a relay that is down now may not be in five minutes. The worker's + // flat backoff is what this classification feeds. + assert.equal(r.retry, true) +}) + +// The other half of the classification, and the one that costs something to get +// wrong in the safe direction: a rejected recipient is not going to be accepted +// on the fifth attempt, and retrying it is four more chances to be seen as a +// sender who ignores bounces. +test('a permanent SMTP refusal is classified terminal, not retried', async () => { + emailConfig.recordStatus = async () => {} + nodemailer.createTransport = () => ({ + sendMail: async () => { + const err = new Error('550 No such user') + err.responseCode = 550 + throw err + }, + }) + emailConfig.getWithSecret = async () => configured() + + const r = await mailer.sendNotification({ to: 'a@b.com', rendered: RENDERED }) + assert.equal(r.ok, false) + assert.equal(r.retry, false) +}) + +// RFC 8058 one-click is only one-click when BOTH headers are present, and the +// header url has to be the API endpoint rather than the page: a client POSTs to +// it without rendering anything. +test('both List-Unsubscribe headers ride along, and the header carries the API url', async () => { + emailConfig.getWithSecret = async () => configured() + const r = await mailer.sendNotification({ + to: 'a@b.com', + rendered: RENDERED, + unsubscribeUrl: 'https://x.test/unsubscribe/tok', + unsubscribeApiUrl: 'https://x.test/api/v1/public/engagement/unsubscribe/tok', + }) + assert.equal(r.ok, true) + assert.equal(sent.headers['List-Unsubscribe'], '') + assert.equal(sent.headers['List-Unsubscribe-Post'], 'List-Unsubscribe=One-Click') +}) + +test('an engagement send is multipart — the html and the text both go out', async () => { + emailConfig.getWithSecret = async () => configured() + await mailer.sendNotification({ to: 'a@b.com', rendered: RENDERED }) + assert.equal(sent.subject, 'A subject') + assert.equal(sent.html, '

body

') + assert.equal(sent.text, 'body') }) diff --git a/server/test/teamBridge.test.js b/server/test/teamBridge.test.js index d40e881..2392cab 100644 --- a/server/test/teamBridge.test.js +++ b/server/test/teamBridge.test.js @@ -143,7 +143,7 @@ test('forums switched off silence the bridge as well as the push', async () => { const result = await teamNotify.forumPost({ team: TEAM, threadId: 41, threadTitle: 'Siege', type: 'discussion', bodyHtml: '

x

', }) - assert.deepEqual(result, { push: 0, emails: 0, bridged: false }) + assert.deepEqual(result, { push: 0, emitted: false, bridged: false }) assert.equal(sent.length, 0) }) diff --git a/server/test/teamNotify.test.js b/server/test/teamNotify.test.js index ccdea1c..b3010ca 100644 --- a/server/test/teamNotify.test.js +++ b/server/test/teamNotify.test.js @@ -263,16 +263,39 @@ test('listPrefs names a Team by its display-name override when staff set one', a // ── 6. The unsubscribe token ─────────────────────────────────────────────── -test('a token verifies for exactly the pair it was signed for', () => { - const token = unsubscribeToken.sign(10, 1) - assert.deepEqual(unsubscribeToken.verify(token), { userId: 10, teamId: 1 }) +test('a v2 token verifies for exactly the channel and scope it was signed for', () => { + const token = unsubscribeToken.sign(10, 'email', 'team:1') + assert.deepEqual(unsubscribeToken.verify(token), { + userId: 10, channel: 'email', scopeKey: 'team:1', version: 2, + }) }) -test('editing the ids in a token invalidates it — the mac covers them', () => { - const token = unsubscribeToken.sign(10, 1) - const [v, uid, tid, mac] = token.split('.') - assert.equal(unsubscribeToken.verify(`${v}.99.${tid}.${mac}`), null) - assert.equal(unsubscribeToken.verify(`${v}.${uid}.99.${mac}`), null) +// The whole point of keeping v1: a link in a mailbox from before Phase 6 must +// still work. It reads as the email channel because an email is the only place a +// v1 token can ever have been. +test('a v1 token still verifies, and reads as the email channel for that Team', () => { + const legacy = unsubscribeToken.signLegacy(10, 1) + assert.deepEqual(unsubscribeToken.verify(legacy), { + userId: 10, channel: 'email', scopeKey: 'team:1', version: 1, + }) +}) + +test('editing the fields in a token invalidates it — the mac covers them', () => { + const [v, uid, channel, scope, mac] = unsubscribeToken.sign(10, 'email', 'team:1').split('.') + assert.equal(unsubscribeToken.verify(`${v}.99.${channel}.${scope}.${mac}`), null) + assert.equal(unsubscribeToken.verify(`${v}.${uid}.${channel}.team:99.${mac}`), null) + assert.equal(unsubscribeToken.verify(`${v}.${uid}.push.${scope}.${mac}`), null) + + const [lv, luid, ltid, lmac] = unsubscribeToken.signLegacy(10, 1).split('.') + assert.equal(unsubscribeToken.verify(`${lv}.99.${ltid}.${lmac}`), null) + assert.equal(unsubscribeToken.verify(`${lv}.${luid}.99.${lmac}`), null) +}) + +// A channel id may legally contain a dot (`discord.dm`, §3.1's own example) and +// the token format's separator is a dot. Refused at signing rather than signed +// into something that verifies as a different channel. +test('a channel id the format cannot carry is refused at signing, not mangled', () => { + assert.throws(() => unsubscribeToken.sign(10, 'discord.dm', 'team:1'), /cannot be carried/) }) test('a garbage token and a well-formed forgery both verify as null', () => { @@ -283,7 +306,7 @@ test('a garbage token and a well-formed forgery both verify as null', () => { }) test('a version bump is what invalidates every outstanding link at once', () => { - const token = unsubscribeToken.sign(10, 1) - const [, uid, tid, mac] = token.split('.') - assert.equal(unsubscribeToken.verify(`${unsubscribeToken.VERSION + 1}.${uid}.${tid}.${mac}`), null) + const [, uid, channel, scope, mac] = unsubscribeToken.sign(10, 'email', 'team:1').split('.') + const next = unsubscribeToken.VERSION + 1 + assert.equal(unsubscribeToken.verify(`${next}.${uid}.${channel}.${scope}.${mac}`), null) }) diff --git a/server/test/teamNotifyDispatch.test.js b/server/test/teamNotifyDispatch.test.js index 1665247..f753a96 100644 --- a/server/test/teamNotifyDispatch.test.js +++ b/server/test/teamNotifyDispatch.test.js @@ -1,17 +1,24 @@ -// The Team notification fan-out and the digest worker (TEAMS.md §6.2/§6.4). +// The Team notification fan-out and the digest worker (TEAMS.md §6.2/§6.4, +// migrated onto the engagement engine in ENGAGEMENT.md Phase 6). // // The layer above teamNotify.test.js: that one asserts WHO a recipient set // contains, this one asserts what actually happens to them — which stream fires, -// what a mail carries, and the four ways a notification is correctly suppressed. +// what leaves for the engine, and the ways a notification is correctly suppressed. // -// The suppressions are the point. A notification feature is mostly refusals, and -// each of these is one that would be invisible until it went wrong in production: +// **What Phase 6 changed in this file, and what it deliberately did not.** The +// immediate email is no longer sent from here: `forumPost` emits an event and the +// engine decides. So the assertions about mail bodies moved down to the email +// channel's own tests, and what is asserted here is the ENVELOPE — the recipient +// set, the scope, the payload — because that is now the whole of this file's +// contract with the rest of the system. Every suppression test survives unchanged +// in intent, and each one is a refusal that would be invisible until it went +// wrong in production: // // • forums switched off silences forum notifications, including the digest; -// • no email configured means the sink is absent, not broken; +// • no enabled rule means Team email is off (Phase 6, decision 3); // • a Team's FIRST roster does not wake 155 phones; -// • a failed send does not stamp `last_digest_at`, so the window is retried -// rather than silently skipped. +// • a failed send does not stamp the digest window, so it is retried rather +// than silently skipped. const { test, beforeEach, afterEach } = require('node:test') const assert = require('node:assert/strict') @@ -19,8 +26,13 @@ const notify = require('../src/utils/teamNotify') const digest = require('../src/utils/teamDigestWorker') const pushDispatch = require('../src/utils/pushDispatch') const mailer = require('../src/utils/mailer') +const engagementEmit = require('../src/utils/engagementEmit') const forumSettings = require('../src/model/teams/teamForumSettings.model') const notifyModel = require('../src/model/teams/teamNotify.model') +const digestDb = require('../src/model/engagement/engagementDigest.db') +const rulesDb = require('../src/model/engagement/engagementRules.db') +const sendsDb = require('../src/model/engagement/engagementSends.db') +const templates = require('../src/engagement/templates') const registries = require('../src/modules/registries') const saved = new Map() @@ -38,27 +50,61 @@ function restore() { const TEAM = { id: 1, slug: 'silver-hand', name: 'The Silver Hand', external_id: 'g1', display_name_override: null } -let sent // tickles -let mails // emails +// The rule the digest worker's gate looks for. Enabled and naming the email +// channel, which is exactly what core does NOT seed — see the gate's own test. +const EMAIL_RULE = { + id: 4, + trigger_id: 'team.forum.post', + enabled: 1, + channels: ['email'], + template_keys: { email: 'notify.team-post', digest: 'notify.digest' }, +} + +let sent // tickles +let emitted // envelopes handed to the engine +let mails // rendered messages handed to the transport let world -function stub({ forumsEnabled = true, emailConfigured = true, recipients = [10, 11], emailRows = [] } = {}) { +function stub({ + forumsEnabled = true, + emailConfigured = true, + recipients = [10, 11], + emailRows = [], + sendOk = true, +} = {}) { sent = [] + emitted = [] mails = [] - world = { stamped: [] } + world = { stamped: [], logged: [] } patch(forumSettings, 'forumsEnabled', async () => forumsEnabled) patch(mailer, 'isConfigured', async () => emailConfigured) - patch(mailer, 'sendTeamNotification', async (msg) => { + patch(mailer, 'sendNotification', async (msg) => { mails.push(msg) - return { sent: true } + return sendOk ? { ok: true, transport: 'smtp' } : { ok: false, retry: true, detail: 'relay said no' } }) patch(pushDispatch, 'publishToUsers', async (streamId, payload) => { sent.push({ streamId, ...payload }) }) + patch(engagementEmit, 'emit', (owner, triggerId, envelope) => { + emitted.push({ owner, triggerId, ...envelope }) + return { ok: true } + }) patch(notifyModel, 'recipientIds', async (teamId, { exclude = [] } = {}) => recipients.filter((id) => !exclude.includes(id))) patch(notifyModel, 'emailRecipients', async (teamId, { exclude = [] } = {}) => emailRows.filter((r) => !exclude.includes(r.user_id))) - patch(notifyModel, 'stampDigest', async (userId, teamId, at) => { world.stamped.push({ userId, teamId, at }) }) + patch(digestDb, 'stampsFor', async () => new Map()) + patch(digestDb, 'stamp', async (userId, channel, scopeKey, at) => { + world.stamped.push({ userId, channel, scopeKey, at }) + }) + patch(rulesDb, 'enabledForTrigger', async (triggerId) => (triggerId === EMAIL_RULE.trigger_id ? [EMAIL_RULE] : [])) + patch(sendsDb, 'record', async (entry) => { world.logged.push(entry) }) + patch(templates, 'renderByKey', async (key, values) => ({ + subject: `[${key}] ${values.periodLabel || values.title || ''}`, + html: '

rendered

', + text: 'rendered', + missing: [], + values, + })) // No module registered: the default in most tests, so the link-building ones // have to opt in and the absence is exercised rather than assumed. patch(registries, 'registeredTeamProvider', () => null) @@ -73,6 +119,9 @@ test('a discussion reply fires team.forum.post; an announcement fires its own st await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) await notify.forumPost({ team: TEAM, threadId: 8, threadTitle: 'Notice', type: 'announcement', authorUserId: 10 }) assert.deepEqual(sent.map((s) => s.streamId), ['team.forum.post', 'team.announcement']) + // The trigger and the stream are the same id under §7.2's one namespace, so + // the event that leaves for the engine names the same thing the tickle did. + assert.deepEqual(emitted.map((e) => e.triggerId), ['team.forum.post', 'team.announcement']) }) test('the tickle is content-free and refs the thread, never the body', async () => { @@ -85,9 +134,12 @@ test('the tickle is content-free and refs the thread, never the body', async () assert.equal(JSON.stringify(sent[0]).includes('private text'), false) }) -test('the author is not among the tickled', async () => { +test('the author is not among the tickled, nor among the emitted audience', async () => { await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) assert.deepEqual(sent[0].userIds, [11]) + // The same exclusion, on the same set: a forum that emails you your own post is + // the first thing anyone turns off. + assert.deepEqual(emitted[0].recipientUserIds, [11]) }) test('roster events fire one tickle for the run, not one per member', async () => { @@ -97,6 +149,66 @@ test('roster events fire one tickle for the run, not one per member', async () = assert.equal(sent.every((s) => s.ref === 'team:1'), true) }) +// The tickle is per run and the EVENT is per person, and that split is the +// declaration's doing: `memberName` is a required single value, so five joiners +// cannot honestly be one event. The rule's cooldown is what stops five mails. +test('a roster sweep emits one event per joiner while tickling once', async () => { + await notify.memberJoined(TEAM, { count: 2, names: ['Darrow', 'Marisol'] }) + assert.equal(sent.length, 1) + assert.deepEqual(emitted.map((e) => e.data.memberName), ['Darrow', 'Marisol']) +}) + +test('a leadership run with only demotions tickles but names no new leader', async () => { + await notify.leadershipChanged(TEAM, { names: [] }) + assert.equal(sent.length, 1) + assert.equal(emitted.length, 0) +}) + +// ── What the envelope carries ────────────────────────────────────────────── + +test('the event carries the access-checked audience, the scope and the team name', async () => { + await notify.forumPost({ + team: TEAM, threadId: 7, threadTitle: 'Raid', type: 'discussion', authorUserId: 10, + authorName: 'Ten', bodyHtml: '

tonight

', + }) + const event = emitted[0] + assert.equal(event.owner, 'core') + assert.deepEqual(event.recipientUserIds, [11]) + // The SCOPE is the id, not the name: an unsubscribe token is signed over it and + // sits in a mailbox for months, and a Team renamed in between must not orphan + // the link. The name rides in the payload, where it is displayed and not keyed. + assert.equal(event.scopeKey, 'team:1') + assert.equal(event.data.teamName, 'The Silver Hand') + assert.equal(event.data.threadTitle, 'Raid') + assert.equal(event.data.excerpt, 'tonight') +}) + +test('an announcement names its title, a post names its thread title', async () => { + await notify.forumPost({ team: TEAM, threadId: 8, threadTitle: 'Notice', type: 'announcement', authorUserId: 10 }) + // The two declarations differ here and a template can only name one of them — + // which is why the seeded announcement rule points at the generic body. Pinned + // so that reconciling the two declarations is a deliberate act with a version + // bump, not a silent rename that empties somebody's subject line. + assert.equal(emitted[0].data.title, 'Notice') + assert.equal(emitted[0].data.threadTitle, undefined) +}) + +test('the post url on the envelope is site-RELATIVE, as the emit contract requires', async () => { + patch(registries, 'registeredTeamProvider', () => ({ pageUrlTemplate: '/uo/guilds/{externalId}' })) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + // An absolute one would be refused by `engagementEmit.RELATIVE_URL`, which + // exists so a `url` variable cannot carry a recipient off-site. The renderer + // absolutizes it against the deployment's base at send time. + assert.equal(emitted[0].data.postUrl, '/uo/guilds/g1?thread=7') +}) + +test('one post is one event however often the path re-runs', async () => { + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.equal(emitted[0].dedupeKey, emitted[1].dedupeKey) + assert.match(emitted[0].dedupeKey, /^team:1:thread:7:/) +}) + // ── Suppression ──────────────────────────────────────────────────────────── test('forums switched off silences a forum notification entirely', async () => { @@ -105,8 +217,9 @@ test('forums switched off silences a forum notification entirely', async () => { // `bridged` is phase 8's third sink (§7.2). Asserted as part of the shape // rather than ignored: "forums are off" has to silence every sink, and a test // that only checked two would not notice a third one still firing. - assert.deepEqual(res, { push: 0, emails: 0, bridged: false }) + assert.deepEqual(res, { push: 0, emitted: false, bridged: false }) assert.equal(sent.length, 0) + assert.equal(emitted.length, 0) }) test('a roster tickle survives forums being off — it is not forum content', async () => { @@ -115,59 +228,27 @@ test('a roster tickle survives forums being off — it is not forum content', as assert.equal(sent.length, 1) }) -test('no recipients means no publish call at all', async () => { +test('no recipients means no publish call and no event at all', async () => { stub({ recipients: [] }) await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) assert.equal(sent.length, 0) + // An event with an empty audience would enqueue nothing anyway; not emitting it + // keeps the send log and the emit log free of rows about nobody. + assert.equal(emitted.length, 0) }) test('a fan-out never throws, whatever the layer below does', async () => { patch(notifyModel, 'recipientIds', async () => { throw new Error('database is on fire') }) const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) - assert.deepEqual(res, { push: 0, emails: 0, bridged: false }) + assert.deepEqual(res, { push: 0, emitted: false, bridged: false }) assert.equal(await notify.memberJoined(TEAM), 0) }) -// ── Email, the immediate mode ────────────────────────────────────────────── - -const IMMEDIATE = [{ user_id: 11, username: 'eleven', email: 'e@example.test', email_mode: 'immediate' }] - -test('only the immediate-mode recipients are emailed per event', async () => { - stub({ - emailRows: [ - ...IMMEDIATE, - { user_id: 12, username: 'twelve', email: 'd@example.test', email_mode: 'digest' }, - { user_id: 13, username: 'thirteen', email: 'o@example.test', email_mode: 'off' }, - ], - }) - await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10, bodyHtml: '

hello

' }) - assert.deepEqual(mails.map((m) => m.to), ['e@example.test']) -}) - -test('an email carries an excerpt and never the whole post', async () => { - stub({ emailRows: IMMEDIATE }) - const long = `

${'x'.repeat(500)}

` - await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10, bodyHtml: long }) - const body = mails[0].items[0].excerpt - assert.equal(body.length < 250, true) - assert.equal(body.endsWith('…'), true) -}) - -test('no email configured means no send and no recipient query', async () => { - stub({ emailConfigured: false, emailRows: IMMEDIATE }) +test('a refused emit is swallowed — a contract bug must not fail a forum write', async () => { + patch(engagementEmit, 'emit', () => { throw new Error('ctx.events.emit: payload is invalid') }) const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) - assert.equal(res.emails, 0) - assert.equal(mails.length, 0) -}) - -test('every email carries an unsubscribe url for that recipient and that Team', async () => { - stub({ emailRows: IMMEDIATE }) - await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) - assert.equal(typeof mails[0].unsubscribeUrl, 'string') - // The header one is an API endpoint (a one-click client POSTs to it without - // rendering anything); the body one is the site's page. They must differ. - assert.notEqual(mails[0].unsubscribeUrl, mails[0].unsubscribeApiUrl) - assert.match(mails[0].unsubscribeApiUrl, /\/api\/v1\/public\/teams\/unsubscribe\//) + assert.equal(res.emitted, false) + assert.equal(res.push, 2 - 1) // the tickle still went out }) // ── Links, and the module that supplies them ─────────────────────────────── @@ -175,12 +256,16 @@ test('every email carries an unsubscribe url for that recipient and that Team', test('with no module-supplied template there is no Team link, and nothing breaks', async () => { assert.equal(notify.teamPageUrl(TEAM), null) assert.equal(notify.threadUrl(TEAM, 7), null) + assert.equal(notify.teamPagePath(TEAM), null) }) test('a registered template becomes an absolute link, and a thread deep-links by search param', () => { patch(registries, 'registeredTeamProvider', () => ({ pageUrlTemplate: '/uo/guilds/{externalId}' })) assert.match(notify.teamPageUrl(TEAM), /\/uo\/guilds\/g1$/) assert.match(notify.threadUrl(TEAM, 7), /\/uo\/guilds\/g1\?thread=7$/) + // The path form is what travels on an envelope; the absolute form is what the + // Discord bridge posts, because a Discord client has no notion of this origin. + assert.equal(notify.teamPagePath(TEAM), '/uo/guilds/g1') }) test('a Team is labelled by its display-name override where staff set one', () => { @@ -190,15 +275,16 @@ test('a Team is labelled by its display-name override where staff set one', () = // ── The digest worker ────────────────────────────────────────────────────── -const DIGEST_ROW = { user_id: 11, username: 'eleven', email: 'e@example.test', email_mode: 'digest', last_digest_at: null } +const DIGEST_ROW = { user_id: 11, username: 'eleven', email: 'e@example.test', email_mode: 'digest' } -function stubDigest({ posts = [], teams = [TEAM], rows = [DIGEST_ROW], sendOk = true } = {}) { +function stubDigest({ posts = [], teams = [TEAM], rows = [DIGEST_ROW], sendOk = true, stamps = new Map() } = {}) { patch(notifyModel, 'teamsWithForumActivitySince', async () => teams) patch(notifyModel, 'emailRecipients', async () => rows) patch(notifyModel, 'digestPostsSince', async () => posts) - patch(mailer, 'sendTeamNotification', async (msg) => { + patch(digestDb, 'stampsFor', async () => stamps) + patch(mailer, 'sendNotification', async (msg) => { mails.push(msg) - return { sent: sendOk } + return sendOk ? { ok: true, transport: 'smtp' } : { ok: false, retry: true, detail: 'relay said no' } }) } @@ -212,7 +298,18 @@ test('a digest gathers the Team’s new posts into one mail', async () => { const res = await digest.tick(new Date()) assert.equal(res.sent, 1) assert.equal(mails.length, 1) - assert.equal(mails[0].items.length, 2) + assert.equal(mails[0].rendered.values.items.length, 2) + assert.equal(mails[0].to, 'e@example.test') +}) + +// §4.6.1: the digest's body is an operator-editable template, not a literal in +// `mailer`. The rule's `template_keys.digest` chooses it, and it is NOT +// `template_keys.email` — that one is written for a single event and would render +// a day of posts as one missing variable. +test('the digest renders the rule’s digest template, not its instant one', async () => { + stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] }) + await digest.tick(new Date()) + assert.match(mails[0].rendered.subject, /^\[notify\.digest\]/) }) test('a recipient with nothing new gets no mail and no stamp', async () => { @@ -223,18 +320,26 @@ test('a recipient with nothing new gets no mail and no stamp', async () => { assert.equal(world.stamped.length, 0, 'stamping here would move the window past unsent posts') }) -test('a failed send leaves last_digest_at alone so the window is retried', async () => { +test('a failed send leaves the digest window alone so it is retried', async () => { stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], sendOk: false }) const res = await digest.tick(new Date()) assert.equal(res.sent, 0) assert.equal(world.stamped.length, 0) + // Failed, and recorded as failed. G15's question is "did user X get it?", and + // "no, and here is why" is an answer the send log has to be able to give. + assert.equal(world.logged[0].status, 'failed') }) -test('a successful send stamps exactly that (user, Team)', async () => { +test('a successful send stamps exactly that (user, channel, scope)', async () => { stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] }) const now = new Date() await digest.tick(now) - assert.deepEqual(world.stamped, [{ userId: 11, teamId: 1, at: now }]) + // The stamp moved out of `team_notification_prefs.last_digest_at` and into + // `engagement_digest_state` (§4.2b), keyed by channel and scope so a second + // digest needs no second column on somebody's preferences row. + assert.deepEqual(world.stamped, [{ userId: 11, channel: 'email', scopeKey: 'team:1', at: now }]) + assert.equal(world.logged[0].status, 'sent') + assert.equal(world.logged[0].outbox_id, null, 'a digest has no outbox row, and the null says so') }) test('only digest-mode recipients are swept', async () => { @@ -242,14 +347,91 @@ test('only digest-mode recipients are swept', async () => { posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], rows: [ DIGEST_ROW, - { user_id: 12, username: 'twelve', email: 'i@example.test', email_mode: 'immediate', last_digest_at: null }, - { user_id: 13, username: 'thirteen', email: 'o@example.test', email_mode: 'off', last_digest_at: null }, + { user_id: 12, username: 'twelve', email: 'i@example.test', email_mode: 'immediate' }, + { user_id: 13, username: 'thirteen', email: 'o@example.test', email_mode: 'off' }, ], }) await digest.tick(new Date()) assert.deepEqual(mails.map((m) => m.to), ['e@example.test']) }) +// The acceptance criterion, verbatim: a pre-existing `email_mode='digest'` row +// produces exactly one daily digest with the same window clamping. The stamp is +// the one the schema backfilled out of `team_notification_prefs`. +test('a pre-existing digest subscriber gets one digest, over the window their old stamp defines', async () => { + const now = new Date('2026-08-18T12:00:00Z') + const yesterday = new Date('2026-08-17T12:00:00Z') + let asked = null + stubDigest({ + posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], + stamps: new Map([[11, yesterday]]), + }) + patch(notifyModel, 'digestPostsSince', async (teamId, since) => { + asked = since + return [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] + }) + const res = await digest.tick(now) + assert.equal(res.sent, 1) + assert.equal(mails.length, 1) + assert.equal(asked.getTime(), yesterday.getTime()) +}) + +// ── The three compute-at-send-time properties (§4.2b) ────────────────────── +// +// Each gets its own named test, the way the Teams phase-5 work gave the +// leader-can't-see-reports rule its own. They are the reason a digest is NOT +// assembled out of outbox rows, and a refactor that "unified" the two paths +// would break all three at once and pass every other test in this file. + +test('property 1 — a two-day outage sends ONE digest, not two days of replay', async () => { + const now = new Date('2026-08-18T12:00:00Z') + stubDigest({ + posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], + stamps: new Map([[11, new Date('2026-08-16T12:00:00Z')]]), + }) + const res = await digest.tick(now) + assert.equal(res.sent, 1) + assert.equal(mails.length, 1) +}) + +test('property 2 — a post hidden after it was written is not in the query, so not in the mail', async () => { + // The moderator hid it, so `digestPostsSince` no longer returns it. There is + // nothing for the worker to remember to remove, which is the property: an + // outbox row snapshotted at publish time would still be carrying the text. + stubDigest({ posts: [] }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 0) + assert.equal(mails.length, 0) +}) + +test('property 3 — a user who lost access between the post and the send is not mailed', async () => { + // `emailRecipients` asks the same two tables the access resolver asks, at SEND + // time. A revoked guest is simply not in the answer. This is the one that would + // have been a security bug: the posts still exist and are still in the window. + stubDigest({ + posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], + rows: [], + }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 0) + assert.equal(mails.length, 0) +}) + +// ── The gate (Phase 6, decision 3) ───────────────────────────────────────── + +test('no enabled email rule means no digest — the operator has not turned Team email on', async () => { + stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] }) + patch(rulesDb, 'enabledForTrigger', async () => []) + assert.equal((await digest.tick(new Date())).skipped, 'no-enabled-rule') + assert.equal(mails.length, 0) +}) + +test('a rule enabled on a channel other than email does not turn the digest on', async () => { + stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] }) + patch(rulesDb, 'enabledForTrigger', async () => [{ ...EMAIL_RULE, channels: ['inapp'] }]) + assert.equal((await digest.tick(new Date())).skipped, 'no-enabled-rule') +}) + test('the sweep is skipped whole when forums are off or email is unconfigured', async () => { stub({ forumsEnabled: false }) assert.equal((await digest.tick(new Date())).skipped, 'forums-disabled')