From e4306d57168508d04b8be3bbfb8c21793baffc32 Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Thu, 30 Apr 2026 18:29:43 +0000 Subject: [PATCH] Send executive-meeting notification emails for real (SMTP delivery) Original task #163: replace the no-op outbox-log behaviour in `sendExecutiveMeetingEmail` with real SMTP delivery so approvers actually get pinged in their inbox when an executive-meeting request is awaiting review. Implementation - Added `nodemailer` (and `@types/nodemailer`) as a dependency of `@workspace/api-server`. nodemailer was already in the build's external list, so the bundle stays slim and resolves it at runtime. - Rewrote `sendExecutiveMeetingEmail` in `artifacts/api-server/src/lib/executive-meeting-notify.ts`: - Lazily builds a cached nodemailer transporter from `SMTP_HOST` / `SMTP_PORT` (default 587) / `SMTP_USER` / `SMTP_PASS`, with optional `SMTP_SECURE` and `SMTP_FROM`. Cache is keyed on a config signature, so changing env vars in tests / hot reloads naturally rebuilds the transporter. - When `SMTP_HOST` is unset the previous outbox-style log is kept verbatim as a fallback. Once `SMTP_HOST` is set the fallback branch is no longer reached. - Picks subject/body in the recipient's preferred language (Arabic vs English) with a sensible fallback. - Sends one mail per addressed recipient in parallel; per-recipient failures are caught and logged at warn level. The outer try/catch keeps the function from ever throwing into the transaction caller. SMTP `rejected` arrays are also logged at warn so bounces are visible. Code-review comment follow-ups (applied in this commit) - Transport cache signature now hashes `SMTP_PASS` (sha256, first 16 hex chars) instead of just "***", so rotating to a new password actually rebuilds the cached transporter without leaking plaintext. - Fallback log message now distinguishes "no SMTP_HOST configured" from "SMTP misconfigured" so operators can tell why a delivery was skipped. Verification - Typecheck passes for the modified file (other unrelated pre-existing typecheck failures remain). - `pnpm --filter @workspace/api-server run build` succeeds. - API server boots cleanly with the new dependency. Deviations / scope - No automated tests added; the existing test harness is integration- style and the project already tracks "Add automated tests for the notification fan-out logic" as a separate task. - Persisting per-recipient delivery state into the `executive_meeting_notifications` audit table and a startup `transporter.verify()` were intentionally deferred and proposed as follow-ups (#232 and #233). Replit-Task-Id: 7362e23d-4531-41a8-a6ec-5f48f85b28c9 --- artifacts/api-server/package.json | 2 + .../src/lib/executive-meeting-notify.ts | 172 +++++++++++++++--- pnpm-lock.yaml | 19 ++ 3 files changed, 167 insertions(+), 26 deletions(-) diff --git a/artifacts/api-server/package.json b/artifacts/api-server/package.json index 535d05e0..785a7137 100644 --- a/artifacts/api-server/package.json +++ b/artifacts/api-server/package.json @@ -25,6 +25,7 @@ "express": "^5", "express-session": "^1.19.0", "google-auth-library": "^10.6.2", + "nodemailer": "^8.0.7", "pdfkit": "^0.18.0", "pino": "^9", "pino-http": "^10", @@ -40,6 +41,7 @@ "@types/express": "^5.0.6", "@types/express-session": "^1.19.0", "@types/node": "catalog:", + "@types/nodemailer": "^8.0.0", "@types/pdfkit": "^0.17.6", "@types/sanitize-html": "^2.16.1", "esbuild": "^0.27.3", diff --git a/artifacts/api-server/src/lib/executive-meeting-notify.ts b/artifacts/api-server/src/lib/executive-meeting-notify.ts index 7a26376f..873cbef2 100644 --- a/artifacts/api-server/src/lib/executive-meeting-notify.ts +++ b/artifacts/api-server/src/lib/executive-meeting-notify.ts @@ -1,4 +1,6 @@ +import { createHash } from "node:crypto"; import { eq, inArray } from "drizzle-orm"; +import nodemailer, { type Transporter } from "nodemailer"; import { db } from "@workspace/db"; import { executiveMeetingNotificationsTable, @@ -169,10 +171,91 @@ export async function getUserIdsForRoleNames( } /** - * Best-effort email side-channel. We don't bundle an SMTP dependency; - * when none is configured we log the outbound message at info level so - * operators can see what would have gone out and wire up real delivery - * later. The function never throws — failure to send email must not + * Lazily-built nodemailer transporter, cached for the life of the + * process. We rebuild it if the relevant SMTP_* env vars change so + * tests / hot-config tweaks don't need a server restart. + */ +type CachedTransporter = { + signature: string; + transporter: Transporter; + from: string; +}; +let cachedTransporter: CachedTransporter | null = null; + +function smtpConfigSignature(): string { + const passFingerprint = process.env.SMTP_PASS + ? createHash("sha256") + .update(process.env.SMTP_PASS) + .digest("hex") + .slice(0, 16) + : ""; + return [ + process.env.SMTP_HOST ?? "", + process.env.SMTP_PORT ?? "", + process.env.SMTP_USER ?? "", + passFingerprint, + process.env.SMTP_SECURE ?? "", + process.env.SMTP_FROM ?? "", + ].join("|"); +} + +function getTransporter(): CachedTransporter | null { + const host = process.env.SMTP_HOST; + if (!host) return null; + const signature = smtpConfigSignature(); + if (cachedTransporter && cachedTransporter.signature === signature) { + return cachedTransporter; + } + const portRaw = process.env.SMTP_PORT; + const port = portRaw ? Number(portRaw) : 587; + if (!Number.isFinite(port) || port <= 0) { + logger.warn( + { portRaw }, + "SMTP_PORT is not a valid number — refusing to build SMTP transporter", + ); + return null; + } + const secureRaw = (process.env.SMTP_SECURE ?? "").toLowerCase(); + const secure = + secureRaw === "true" || secureRaw === "1" || secureRaw === "yes" + ? true + : secureRaw === "false" || secureRaw === "0" || secureRaw === "no" + ? false + : port === 465; + const user = process.env.SMTP_USER; + const pass = process.env.SMTP_PASS; + const transporter = nodemailer.createTransport({ + host, + port, + secure, + auth: user && pass ? { user, pass } : undefined, + }); + const from = + process.env.SMTP_FROM && process.env.SMTP_FROM.length > 0 + ? process.env.SMTP_FROM + : user && user.length > 0 + ? user + : "no-reply@localhost"; + cachedTransporter = { signature, transporter, from }; + return cachedTransporter; +} + +function pickLang( + lang: string | null | undefined, + values: { ar: string | null; en: string | null }, +): string { + const isArabic = + typeof lang === "string" && lang.toLowerCase().startsWith("ar"); + const primary = isArabic ? values.ar : values.en; + const fallback = isArabic ? values.en : values.ar; + return (primary ?? fallback ?? "").trim(); +} + +/** + * Best-effort email side-channel. When SMTP is configured we deliver + * via nodemailer; otherwise we fall back to logging the outbound + * message at info level so operators can see what would have gone + * out. The function never throws — failure to send email must not * roll back a meeting/request mutation. */ export async function sendExecutiveMeetingEmail( @@ -196,7 +279,11 @@ export async function sendExecutiveMeetingEmail( ); if (addressed.length === 0) return; - if (!process.env.SMTP_HOST) { + const cached = getTransporter(); + if (!cached) { + const reason = process.env.SMTP_HOST + ? "SMTP misconfigured — not delivered" + : "no SMTP_HOST configured — not delivered"; logger.info( { notificationType, @@ -208,31 +295,64 @@ export async function sendExecutiveMeetingEmail( subject, body, }, - "executive-meeting email outbox (no SMTP_HOST configured — not delivered)", + `executive-meeting email outbox (${reason})`, ); return; } - // SMTP delivery would go here. We intentionally don't import - // nodemailer at the top of the file so the build stays slim when - // email isn't configured. Once SMTP is wired up, swap this branch - // for a real send. - logger.warn( - { notificationType, count: addressed.length }, - "SMTP_HOST is set but executive-meeting email delivery is not implemented — falling back to outbox log", - ); - logger.info( - { - notificationType, - recipients: addressed.map((r) => ({ - id: r.id, - email: r.email, - lang: r.preferredLanguage, - })), - subject, - body, - }, - "executive-meeting email outbox", + const { transporter, from } = cached; + await Promise.all( + addressed.map(async (r) => { + const langSubject = pickLang(r.preferredLanguage, { + ar: subject.ar, + en: subject.en, + }); + const langBody = pickLang(r.preferredLanguage, body); + try { + const info = await transporter.sendMail({ + from, + to: r.email!, + subject: langSubject, + text: langBody, + }); + const accepted = Array.isArray(info.accepted) ? info.accepted : []; + const rejected = Array.isArray(info.rejected) ? info.rejected : []; + if (rejected.length > 0) { + logger.warn( + { + notificationType, + recipientId: r.id, + email: r.email, + rejected, + response: info.response, + messageId: info.messageId, + }, + "executive-meeting email rejected by SMTP server", + ); + } else { + logger.info( + { + notificationType, + recipientId: r.id, + email: r.email, + accepted, + messageId: info.messageId, + }, + "executive-meeting email sent", + ); + } + } catch (err) { + logger.warn( + { + err, + notificationType, + recipientId: r.id, + email: r.email, + }, + "executive-meeting email send failed", + ); + } + }), ); } catch (err) { logger.warn( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 16b49841..1fa51f2c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -120,6 +120,9 @@ importers: google-auth-library: specifier: ^10.6.2 version: 10.6.2 + nodemailer: + specifier: ^8.0.7 + version: 8.0.7 pdfkit: specifier: ^0.18.0 version: 0.18.0 @@ -160,6 +163,9 @@ importers: '@types/node': specifier: 'catalog:' version: 25.3.5 + '@types/nodemailer': + specifier: ^8.0.0 + version: 8.0.0 '@types/pdfkit': specifier: ^0.17.6 version: 0.17.6 @@ -2657,6 +2663,9 @@ packages: '@types/node@25.3.5': resolution: {integrity: sha512-oX8xrhvpiyRCQkG1MFchB09f+cXftgIXb3a7UUa4Y3wpmZPw5tyZGTLWhlESOLq1Rq6oDlc8npVU2/9xiCuXMA==} + '@types/nodemailer@8.0.0': + resolution: {integrity: sha512-fyf8jWULsCo0d0BuoQ75i6IeoHs47qcqxWc7yUdUcV0pOZGjUTTOvwdG1PRXUDqN/8A64yQdQdnA2pZgcdi+cA==} + '@types/pdfkit@0.17.6': resolution: {integrity: sha512-tIwzxk2uWKp0Cq9JIluQXJid77lYhF52EsIOwhsMF4iWLA6YneoBR1xVKYYdAysHuepUB0OX4tdwMiUDdGKmig==} @@ -3992,6 +4001,10 @@ packages: node-releases@2.0.36: resolution: {integrity: sha512-TdC8FSgHz8Mwtw9g5L4gR/Sh9XhSP/0DEkQxfEFXOpiul5IiHgHan2VhYYb6agDSfp4KuvltmGApc8HMgUrIkA==} + nodemailer@8.0.7: + resolution: {integrity: sha512-pkjE4mkBzQjdJT4/UmlKl3pX0rC9fZmjh7c6C9o7lv66Ac6w9WCnzPzhbPNxwZAzlF4mdq4CSWB5+FbK6FWCow==} + engines: {node: '>=6.0.0'} + npm-run-path@6.0.0: resolution: {integrity: sha512-9qny7Z9DsQU8Ou39ERsPU4OZQlSTP47ShQzuKZ6PRXpYLtIFgl/DEBYEXKlvcEa+9tHVcK8CF81Y2V72qaZhWA==} engines: {node: '>=18'} @@ -6722,6 +6735,10 @@ snapshots: dependencies: undici-types: 7.18.2 + '@types/nodemailer@8.0.0': + dependencies: + '@types/node': 25.3.5 + '@types/pdfkit@0.17.6': dependencies: '@types/node': 25.3.5 @@ -8044,6 +8061,8 @@ snapshots: node-releases@2.0.36: {} + nodemailer@8.0.7: {} + npm-run-path@6.0.0: dependencies: path-key: 4.0.0