diff --git a/docs/inbound-mail-effects.md b/docs/inbound-mail-effects.md index bf68fe92..f780e777 100644 --- a/docs/inbound-mail-effects.md +++ b/docs/inbound-mail-effects.md @@ -247,7 +247,7 @@ non-delivery**: - `assertRoutable` refuses a re-route (409, `SEND_IN_FLIGHT_CONFLICT`) while the newest attempt is in flight — including the `retargeted` branch. `MailService.reply`/`resend` apply the same guard. - `runAutoForwardWith` classifies the deadline error distinctly and does **not** retry it. Retrying would - put a second multi-MB packet in front of a government contact for a message that very likely sent. + put a second packet in front of a government contact for a message that very likely sent. - The operator sees a **409 CONFLICT** with "the send is still in progress", not a 500 — so the console's existing conflict handling applies and the error tracker is not spammed for an expected outcome. @@ -263,8 +263,9 @@ in flight; any other `failed` → failed; no event and older than the stale wind `assertOutboundSendPolicy` runs in `loadEnv`, so the process refuses to boot when the knobs would let one send outlive the guard: `OUTBOUND_SEND_MIN_THROUGHPUT_BPS` must be ≥ 1024, `OCI_EMAIL_SMTP_TIMEOUT_MS` -must be ≤ 60 000, and the largest computable deadline — sized for `MAX_PACKET_TOTAL_BYTES` (8 MiB) after -base64 expansion (+33%, which is what actually crosses the wire) — must fit inside the 900 s window. At +must be ≤ 60 000, and the largest computable deadline — sized for `OUTBOUND_PAYLOAD_BUDGET_BYTES` (8 MiB) +after base64 expansion (+33%), far above any packet now that report media goes out as links rather than +attachments — must fit inside the 900 s window. At the defaults that largest deadline is ≈ 88 s, comfortably inside it. | Env var | Default | Notes | diff --git a/services/api/src/adapters/mailer.oci.ts b/services/api/src/adapters/mailer.oci.ts index e6ab14c4..0db5aca7 100644 --- a/services/api/src/adapters/mailer.oci.ts +++ b/services/api/src/adapters/mailer.oci.ts @@ -80,8 +80,7 @@ function classifyMailError(err: unknown, from: string): MailSendError { case "oversize": return new MailSendError( ErrorCode.CONFLICT, - `Email not sent: the message (with its attachments) is too large for the mail provider. ` + - `Send fewer or smaller photos — the rest remain available as links.${detail}`, + `Email not sent: the message is too large for the mail provider.${detail}`, smtp, { cause: err }, ) diff --git a/services/api/src/routes/admin/mail.routes.ts b/services/api/src/routes/admin/mail.routes.ts index 98c40a2f..ee6e56d9 100644 --- a/services/api/src/routes/admin/mail.routes.ts +++ b/services/api/src/routes/admin/mail.routes.ts @@ -120,7 +120,6 @@ export async function registerAdminMailRoutes( makeMailService({ repo: overrides.repo, outboundMail: overrides.outboundMail, - loadAttachmentBytes: (key) => (overrides.outboundStorage ?? container.storage).getObject(key), }), () => { const sql = container.getDb().sql @@ -129,7 +128,6 @@ export async function registerAdminMailRoutes( return makeMailService({ repo, outboundMail, - loadAttachmentBytes: (key) => container.storage.getObject(key), }) }, ) diff --git a/services/api/src/routes/admin/reports.routes.ts b/services/api/src/routes/admin/reports.routes.ts index edd894c8..93e7553e 100644 --- a/services/api/src/routes/admin/reports.routes.ts +++ b/services/api/src/routes/admin/reports.routes.ts @@ -106,7 +106,6 @@ export async function registerAdminReportsRoutes( presignPacketMedia: makePacketMediaPresigner(container.storage), loadLinkedEventsForReports: (reportIds) => cleanupRepo.loadLinkedEventsForReports(reportIds), - loadMediaBytes: (k) => container.storage.getObject(k), reportChatEmitter: makeContainerReportChatEmitter(container, app.log), forwardTemplates: makeDrizzleForwardTemplateRepository(sql), notifications: makeRouteNotificationService(container, app.log), diff --git a/services/api/src/services/admin/admin-report-service.ts b/services/api/src/services/admin/admin-report-service.ts index aed04591..b45fbc10 100644 --- a/services/api/src/services/admin/admin-report-service.ts +++ b/services/api/src/services/admin/admin-report-service.ts @@ -18,7 +18,6 @@ import type { ReportRouting, ReportTimelineItem, } from "@civfix/shared" -import type { PacketAttachment } from "./outbound-mail-service.js" import { toLinkedEventRef, type LinkedEventView } from "../cleanup-service.js" import { toRelAbs } from "./admin-format.js" import { toPersonDTO } from "./admin-person.js" @@ -30,14 +29,7 @@ import { statusChangeNote, timelineKindForStatus, } from "./admin-report-status.js" -import { - attachmentFilename, - buildReportPacket, - MAX_PACKET_ATTACHMENTS, - MAX_PACKET_ATTACHMENT_BYTES, - MAX_PACKET_TOTAL_BYTES, - type PacketMediaLink, -} from "./mail-format.js" +import { buildReportPacket, type PacketMediaLink } from "./mail-format.js" import { pickPreviewMedia, previewThumbnailUrl } from "./admin-report-types.js" import type { AdminReportMediaRecord, @@ -53,13 +45,7 @@ import type { export * from "./admin-report-types.js" export * from "./admin-report-status.js" -export { - buildReportPacket, - attachmentFilename, - MAX_PACKET_ATTACHMENTS, - MAX_PACKET_ATTACHMENT_BYTES, - type ReportPacket, -} from "./mail-format.js" +export { buildReportPacket, type ReportPacket } from "./mail-format.js" const ROUTABLE_FROM_STATUSES: readonly AdminReportStatus[] = ["submitted", "held", "published"] @@ -395,22 +381,8 @@ export function makeAdminReportService(deps: AdminReportServiceDeps): AdminRepor const publiclyVisible = isPubliclyVisibleStatus(record.status) && record.visibility === "public" const packetMedia: PacketMediaLink[] = [] - const attachments: PacketAttachment[] = [] - let attachedBytesTotal = 0 for (const m of media) { packetMedia.push({ kind: m.kind, url: await presignPacketMedia(m.r2Key, publiclyVisible) }) - if (m.kind !== "image" || attachments.length >= MAX_PACKET_ATTACHMENTS) continue - const bytes = deps.loadMediaBytes ? await deps.loadMediaBytes(m.r2Key) : null - if (bytes === null || bytes.byteLength > MAX_PACKET_ATTACHMENT_BYTES) continue - if (attachedBytesTotal + bytes.byteLength > MAX_PACKET_TOTAL_BYTES) continue - attachedBytesTotal += bytes.byteLength - const contentType = attachmentContentType(m.contentType, bytes) - attachments.push({ - key: m.r2Key, - filename: attachmentFilename(m.r2Key, attachments.length, contentType), - contentType, - content: bytes, - }) } const defaults = @@ -431,7 +403,6 @@ export function makeAdminReportService(deps: AdminReportServiceDeps): AdminRepor subject: packet.subject, text: packet.text, html: packet.html, - ...(attachments.length > 0 ? { attachments } : {}), audit: { actorId: input.actorId, action: "report.routed", @@ -491,36 +462,6 @@ export function makeAdminReportService(deps: AdminReportServiceDeps): AdminRepor } } -export function attachmentContentType( - stored: string | null | undefined, - bytes: Uint8Array, -): string { - if (stored !== null && stored !== undefined && stored.trim() !== "") return stored - return sniffImageMime(bytes) ?? "image/jpeg" -} - -function sniffImageMime(bytes: Uint8Array): string | null { - if (bytes.length >= 3 && bytes[0] === 0xff && bytes[1] === 0xd8 && bytes[2] === 0xff) { - return "image/jpeg" - } - const PNG = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a] - if (bytes.length >= PNG.length && PNG.every((b, i) => bytes[i] === b)) return "image/png" - if ( - bytes.length >= 12 && - bytes[0] === 0x52 && - bytes[1] === 0x49 && - bytes[2] === 0x46 && - bytes[3] === 0x46 && - bytes[8] === 0x57 && - bytes[9] === 0x45 && - bytes[10] === 0x42 && - bytes[11] === 0x50 - ) { - return "image/webp" - } - return null -} - function sameAddress(a: string, b: string): boolean { return a.trim().toLowerCase() === b.trim().toLowerCase() } diff --git a/services/api/src/services/admin/admin-report-types.ts b/services/api/src/services/admin/admin-report-types.ts index 57b7538f..aa09a23a 100644 --- a/services/api/src/services/admin/admin-report-types.ts +++ b/services/api/src/services/admin/admin-report-types.ts @@ -27,7 +27,6 @@ export interface AdminReportMediaRecord { kind: "image" | "video" r2Key: string thumbKey: string | null - contentType?: string | null } export interface AdminReportTimelineRecord { @@ -171,7 +170,6 @@ export interface AdminReportServiceDeps { loadLinkedEventsForReports?: ( reportIds: string[], ) => Promise> - loadMediaBytes?: (r2Key: string) => Promise now?: () => Date reportChatEmitter?: ReportChatSystemEmitter forwardTemplates?: ForwardTemplateReader diff --git a/services/api/src/services/admin/autoforward-jobs.ts b/services/api/src/services/admin/autoforward-jobs.ts index edc79f90..d7a80f50 100644 --- a/services/api/src/services/admin/autoforward-jobs.ts +++ b/services/api/src/services/admin/autoforward-jobs.ts @@ -99,7 +99,6 @@ function makeAutoForwardService( outboundMail, presignMedia: makePrivateMediaPresigner(container.storage), presignPacketMedia: makePacketMediaPresigner(container.storage), - loadMediaBytes: (k) => container.storage.getObject(k), reportChatEmitter: makeContainerReportChatEmitter(container, logger), forwardTemplates: makeDrizzleForwardTemplateRepository(sql), }) diff --git a/services/api/src/services/admin/mail-format.ts b/services/api/src/services/admin/mail-format.ts index a57154ba..e8407d7b 100644 --- a/services/api/src/services/admin/mail-format.ts +++ b/services/api/src/services/admin/mail-format.ts @@ -28,32 +28,6 @@ import type { MarkdownInline } from "@civfix/shared/markdown" export const NO_PHOTO_LINKS = "(none)" -export const MAX_PACKET_ATTACHMENTS = 10 -export const MAX_PACKET_ATTACHMENT_BYTES = 10 * 1024 * 1024 -export const MAX_PACKET_TOTAL_BYTES = 8 * 1024 * 1024 - -const ATTACHMENT_EXTENSIONS: Record = { - "image/jpeg": ".jpg", - "image/png": ".png", - "image/webp": ".webp", -} - -const EXTENSION_FALLBACK = ".jpg" - -function hasExtension(name: string): boolean { - return /\.[A-Za-z0-9]{2,5}$/.test(name) -} - -export function attachmentFilename(r2Key: string, index: number, contentType?: string): string { - const ext = - ATTACHMENT_EXTENSIONS[(contentType ?? "").trim().toLowerCase()] ?? EXTENSION_FALLBACK - const tail = r2Key.split("/").pop() ?? "" - const cleaned = tail.replace(/[^A-Za-z0-9._-]+/g, "_").replace(/^\.+/, "") - if (cleaned.length === 0) return `photo-${index + 1}${ext}` - if (hasExtension(cleaned)) return cleaned.slice(0, 120) - return `${cleaned.slice(0, 120 - ext.length)}${ext}` -} - export interface ReportPacket { subject: string text: string diff --git a/services/api/src/services/admin/mail-repository.drizzle.ts b/services/api/src/services/admin/mail-repository.drizzle.ts index 34cdaf01..147d93ba 100644 --- a/services/api/src/services/admin/mail-repository.drizzle.ts +++ b/services/api/src/services/admin/mail-repository.drizzle.ts @@ -42,7 +42,6 @@ import { type ThreadInit, } from "./mail-repository.js" import type { - MailAttachment, MailDirection, MailStatsResponse, MailStatus, @@ -792,10 +791,9 @@ export function makeDrizzleMailRepository(sql: Sql): MailRepository { subject: string | null body: string | null html: string | null - attachments: MailAttachment[] | null }[] >` - SELECT id, to_addr, subject, body, html, attachments + SELECT id, to_addr, subject, body, html FROM mail_messages WHERE id = ${messageId} AND direction = 'out' LIMIT 1 @@ -808,7 +806,6 @@ export function makeDrizzleMailRepository(sql: Sql): MailRepository { subject: row.subject, body: row.body ?? "", html: row.html, - attachments: row.attachments ?? [], } }, diff --git a/services/api/src/services/admin/mail-repository.memory.ts b/services/api/src/services/admin/mail-repository.memory.ts index e4033c25..2e8e5f9b 100644 --- a/services/api/src/services/admin/mail-repository.memory.ts +++ b/services/api/src/services/admin/mail-repository.memory.ts @@ -596,7 +596,6 @@ export class InMemoryMailRepository implements MailRepository { subject: m.subject, body: m.body ?? "", html: m.html, - attachments: m.attachments, }) } diff --git a/services/api/src/services/admin/mail-repository.ts b/services/api/src/services/admin/mail-repository.ts index 0c78394d..ca04a7af 100644 --- a/services/api/src/services/admin/mail-repository.ts +++ b/services/api/src/services/admin/mail-repository.ts @@ -142,7 +142,6 @@ export interface OutboundMessageSnapshot { subject: string | null body: string html: string | null - attachments: MailAttachment[] } export interface RecordEventInput { diff --git a/services/api/src/services/admin/mail-service.ts b/services/api/src/services/admin/mail-service.ts index 90371dbf..3e04f363 100644 --- a/services/api/src/services/admin/mail-service.ts +++ b/services/api/src/services/admin/mail-service.ts @@ -11,20 +11,12 @@ import type { } from "@civfix/shared" import type { ListThreadsInput, MailRepository } from "./mail-repository.drizzle.js" import { domainOf } from "../../adapters/mail-text.js" -import type { PacketAttachment } from "./outbound-mail-service.js" -import { attachmentContentType, SEND_IN_FLIGHT_CONFLICT } from "./admin-report-service.js" -import { - MAX_PACKET_ATTACHMENTS, - MAX_PACKET_ATTACHMENT_BYTES, - MAX_PACKET_TOTAL_BYTES, -} from "./mail-format.js" -import { mapWithLimit, PRESIGN_CONCURRENCY } from "../media-presign.js" +import { SEND_IN_FLIGHT_CONFLICT } from "./admin-report-service.js" import type { OutboundMailService } from "./outbound-mail-service.js" export interface MailServiceDeps { repo: MailRepository outboundMail: OutboundMailService - loadAttachmentBytes?: (key: string) => Promise } export interface MailService { @@ -41,37 +33,6 @@ export interface MailService { export function makeMailService(deps: MailServiceDeps): MailService { const { repo, outboundMail } = deps - async function replayAttachments( - attachments: readonly { key: string; filename: string }[], - ): Promise { - const load = deps.loadAttachmentBytes - if (load === undefined) return [] - let total = 0 - const loaded = await mapWithLimit( - attachments.slice(0, MAX_PACKET_ATTACHMENTS), - PRESIGN_CONCURRENCY, - async (att) => { - if (total >= MAX_PACKET_TOTAL_BYTES) return null - const bytes = await load(att.key) - if (bytes === null || bytes.byteLength > MAX_PACKET_ATTACHMENT_BYTES) return null - if (total + bytes.byteLength > MAX_PACKET_TOTAL_BYTES) return null - total += bytes.byteLength - return { key: att.key, filename: att.filename, bytes } - }, - ) - const replayed: PacketAttachment[] = [] - for (const att of loaded) { - if (att === null) continue - replayed.push({ - key: att.key, - filename: att.filename, - contentType: attachmentContentType(null, att.bytes), - content: att.bytes, - }) - } - return replayed - } - async function requireThreadDTO(id: string): Promise { const dto = await repo.getThread(id) if (!dto) throw AppError.notFound("Mail thread not found") @@ -171,13 +132,11 @@ export function makeMailService(deps: MailServiceDeps): MailService { throw AppError.validation({ to: "No recipient address on this thread to resend to." }) } const subject = last.subject ?? thread.subject ?? "" - const attachments = await replayAttachments(last.attachments) await outboundMail.appendOutbound(id, { body: last.body, toAddr, ...(subject.length > 0 ? { subject } : {}), ...(last.html !== null ? { html: last.html } : {}), - ...(attachments.length > 0 ? { attachments } : {}), kind: "resend", audit: { actorId, action: "mail.resent", meta: { to: toAddr } }, }) diff --git a/services/api/src/services/admin/outbound-mail-service.ts b/services/api/src/services/admin/outbound-mail-service.ts index 175a3733..28e90252 100644 --- a/services/api/src/services/admin/outbound-mail-service.ts +++ b/services/api/src/services/admin/outbound-mail-service.ts @@ -1,5 +1,5 @@ import { AppError, ErrorCode } from "@civfix/shared" -import type { Mailer, OutboundAttachment, SentMail } from "@civfix/shared/interfaces" +import type { Mailer, SentMail } from "@civfix/shared/interfaces" import type { DbHandle } from "../../db/client.js" import type { Env } from "../../env/types.js" import { @@ -10,10 +10,9 @@ import { type MailRepository, type MailThreadRecord, } from "./mail-repository.drizzle.js" -import type { MailAttachment, MailStatus } from "@civfix/shared" +import type { MailStatus } from "@civfix/shared" import { domainOf } from "../../adapters/mail-text.js" import { - base64Bytes, outboundSendDeadlineMs, phaseBudgetFor, OUTBOUND_SEND_MIN_THROUGHPUT_BPS, @@ -25,10 +24,6 @@ export interface OutboundMailEnv { MAIL_REPLY_DOMAIN: string } -export interface PacketAttachment extends OutboundAttachment { - key?: string -} - export interface OutboundMailLogger { warn(obj: unknown, msg?: string): void } @@ -62,7 +57,6 @@ export interface AppendOutboundInput { toAddr: string subject?: string html?: string - attachments?: PacketAttachment[] kind?: MailMessageKind audit?: OutboundAudit eventMeta?: Record @@ -76,7 +70,6 @@ export interface SendReportInput { subject: string text: string html?: string - attachments?: PacketAttachment[] kind?: MailMessageKind audit?: MailAuditInput } @@ -147,27 +140,10 @@ export function isOutboundSendDeadlineError(err: unknown): boolean { return (err as { outboundSendDeadline?: unknown }).outboundSendDeadline === true } -export function attachmentMetadata( - attachments: readonly PacketAttachment[] | undefined, -): MailAttachment[] { - const stored: MailAttachment[] = [] - for (const att of attachments ?? []) { - if (att.key === undefined || att.key === "") continue - stored.push({ key: att.key, filename: att.filename, size: att.content.byteLength }) - } - return stored -} - -export function outboundPayloadBytes(input: { - body: string - html?: string | undefined - attachments?: readonly OutboundAttachment[] | undefined -}): number { +export function outboundPayloadBytes(input: { body: string; html?: string | undefined }): number { let bytes = Buffer.byteLength(input.body, "utf8") if (input.html !== undefined) bytes += Buffer.byteLength(input.html, "utf8") - let attachmentBytes = 0 - for (const att of input.attachments ?? []) attachmentBytes += att.content.byteLength - return bytes + base64Bytes(attachmentBytes) + return bytes } function raceDeadline(work: Promise, ms: number): Promise { @@ -202,7 +178,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound subject: string body: string html?: string - attachments?: PacketAttachment[] eventMeta?: Record onLateSuccess?: (() => Promise) | undefined }): Promise { @@ -211,11 +186,7 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound const inReplyTo = priorIds.length > 0 ? priorIds[priorIds.length - 1] : undefined const references = priorIds.length > 10 ? [priorIds[0] as string, ...priorIds.slice(-9)] : priorIds - const bytes = outboundPayloadBytes({ - body: args.body, - html: args.html, - attachments: args.attachments, - }) + const bytes = outboundPayloadBytes({ body: args.body, html: args.html }) const deadlineMs = deps.sendDeadlineMs ?? outboundSendDeadlineMs({ @@ -233,7 +204,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound messageId: rfcMessageId, ...(inReplyTo !== undefined ? { inReplyTo } : {}), ...(references.length > 0 ? { references } : {}), - ...(args.attachments !== undefined ? { attachments: args.attachments } : {}), }) async function recordFailed(err: unknown, extra: Record): Promise { @@ -397,7 +367,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound thread.subject = input.subject } const fromHeader = fromHeaderForThread(thread) - const attachments = attachmentMetadata(input.attachments) const message = await insertOut({ threadId: thread.id, direction: "out", @@ -407,7 +376,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound body: input.text, kind: input.kind ?? "packet", ...(input.html !== undefined ? { html: input.html } : {}), - ...(attachments.length > 0 ? { attachments } : {}), ...(input.audit ? { audit: { @@ -429,7 +397,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound subject: input.subject, body: input.text, ...(input.html !== undefined ? { html: input.html } : {}), - ...(input.attachments !== undefined ? { attachments: input.attachments } : {}), eventMeta: { reportId: input.reportId, ...(input.geoid != null ? { geoid: input.geoid } : {}), @@ -565,7 +532,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound } const subject = input.subject ?? replySubject(thread.subject) const fromHeader = fromHeaderForThread(thread) - const attachments = attachmentMetadata(input.attachments) const message = await insertOut({ threadId: thread.id, direction: "out", @@ -575,7 +541,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound body: input.body, kind: input.kind ?? "reply", ...(input.html !== undefined ? { html: input.html } : {}), - ...(attachments.length > 0 ? { attachments } : {}), ...(input.audit ? { audit: { ...input.audit, target: `mail:${thread.id}` } } : {}), }) await deliverAndRecord({ @@ -586,7 +551,6 @@ export function makeOutboundMailService(deps: OutboundMailServiceDeps): Outbound subject, body: input.body, ...(input.html !== undefined ? { html: input.html } : {}), - ...(input.attachments !== undefined ? { attachments: input.attachments } : {}), ...(input.eventMeta !== undefined ? { eventMeta: input.eventMeta } : {}), }) return freshThread(thread) diff --git a/services/api/src/services/admin/outbound-send-policy.ts b/services/api/src/services/admin/outbound-send-policy.ts index 96bc25b0..2f4a34c2 100644 --- a/services/api/src/services/admin/outbound-send-policy.ts +++ b/services/api/src/services/admin/outbound-send-policy.ts @@ -1,7 +1,7 @@ -import { MAX_PACKET_TOTAL_BYTES } from "./mail-format.js" - export const OUTBOUND_SEND_PHASE_BUDGET_MS = 45_000 +export const OUTBOUND_PAYLOAD_BUDGET_BYTES = 8 * 1024 * 1024 + export const OUTBOUND_SEND_MIN_THROUGHPUT_BPS = 256 * 1024 export const OUTBOUND_SEND_MIN_THROUGHPUT_FLOOR_BPS = 1024 @@ -41,7 +41,7 @@ export function maxOutboundSendDeadlineMs(input: { minThroughputBytesPerSec: number }): number { return outboundSendDeadlineMs({ - bytes: base64Bytes(MAX_PACKET_TOTAL_BYTES), + bytes: base64Bytes(OUTBOUND_PAYLOAD_BUDGET_BYTES), phaseBudgetMs: phaseBudgetFor(input.smtpTimeoutMs), minThroughputBytesPerSec: input.minThroughputBytesPerSec, }) diff --git a/services/api/test/unit/admin-mail-repository.test.ts b/services/api/test/unit/admin-mail-repository.test.ts index 83c753b7..8e9d57da 100644 --- a/services/api/test/unit/admin-mail-repository.test.ts +++ b/services/api/test/unit/admin-mail-repository.test.ts @@ -488,7 +488,6 @@ describe("InMemoryMailRepository: outbound snapshot + failure recording", () => subject: "civfix report: Pothole", body: "second", html: "

second

", - attachments: [{ key: "media/r2/a.jpg", filename: "a.jpg", size: 4 }], }) }) diff --git a/services/api/test/unit/admin-mail.test.ts b/services/api/test/unit/admin-mail.test.ts index d8189969..dc995991 100644 --- a/services/api/test/unit/admin-mail.test.ts +++ b/services/api/test/unit/admin-mail.test.ts @@ -15,24 +15,18 @@ interface Harness { repo: InMemoryMailRepository mailer: FakeMailer svc: MailService - objects: Map } function harness(): Harness { const repo = new InMemoryMailRepository() const mailer = new FakeMailer() - const objects = new Map() const outboundMail = makeOutboundMailService({ repo, mailer, env: { MAIL_FROM_OUTREACH: FROM_OUTREACH, MAIL_REPLY_DOMAIN: "civfix.org" }, }) - const svc = makeMailService({ - repo, - outboundMail, - loadAttachmentBytes: (key) => Promise.resolve(objects.get(key) ?? null), - }) - return { repo, mailer, svc, objects } + const svc = makeMailService({ repo, outboundMail }) + return { repo, mailer, svc } } describe("mail-service recipient-resolution helpers", () => { @@ -372,10 +366,9 @@ describe("mail-service: resend", () => { expect(mailer.sent.at(-1)?.to).toBe("mayor@city.gov") }) - it("replays the untruncated body, the html part and the stored attachments", async () => { + it("replays the untruncated body and the html part, never a stored attachment", async () => { const h = harness() const body = "x".repeat(100_000) - h.objects.set("media/r2/photo.jpg", new Uint8Array([0xff, 0xd8, 0xff, 0x01])) const t = await h.repo.createThread({ subject: "Pothole", org: "City of LA" }) await h.repo.insertMessage({ threadId: t.id, @@ -398,12 +391,11 @@ describe("mail-service: resend", () => { expect(sent?.text).toBe(body) expect(sent?.html).toBe("

packet

") expect(sent?.subject).toBe("civfix report: Pothole") - expect(sent?.attachments?.map((a) => a.filename)).toEqual(["photo.jpg"]) - expect(sent?.attachments?.[0]?.contentType).toBe("image/jpeg") + expect(sent?.attachments).toBeUndefined() const replayed = h.repo.messagesOf(t.id).at(-1) expect(replayed?.kind).toBe("resend") expect(replayed?.html).toBe("

packet

") - expect(replayed?.attachments).toEqual([{ key: "media/r2/photo.jpg", filename: "photo.jpg", size: 4 }]) + expect(replayed?.attachments).toEqual([]) }) it("404s an unknown thread and 422s a thread with no outbound message to resend", async () => { diff --git a/services/api/test/unit/admin-reports.test.ts b/services/api/test/unit/admin-reports.test.ts index b34845d0..c1895e21 100644 --- a/services/api/test/unit/admin-reports.test.ts +++ b/services/api/test/unit/admin-reports.test.ts @@ -18,7 +18,6 @@ import { import { InMemoryMailRepository } from "../../src/services/admin/mail-repository.memory.js" import { InMemoryForwardTemplateRepository } from "../../src/services/admin/forward-template-repository.memory.js" import { RecordingNotifier } from "../helpers/notifications.js" -import { MAX_PACKET_TOTAL_BYTES } from "../../src/services/admin/mail-format.js" import { makeOutboundMailService, OutboundSendDeadlineError, @@ -1457,10 +1456,8 @@ describe("F009 routeToJurisdiction concurrent double-send guard", () => { }) }) -describe("F108 report-packet attachments are bounded in AGGREGATE, not just per file", () => { - const FOUR_MB = 4 * 1024 * 1024 - - function harnessWithMedia(count: number, bytesEach: number) { +describe("report packets carry media as links, never as attachments", () => { + function harnessWithMedia() { const repo = new InMemoryAdminReportRepository() repo.now = NOW const mailRepo = new InMemoryMailRepository() @@ -1470,16 +1467,11 @@ describe("F108 report-packet attachments are bounded in AGGREGATE, not just per mailer, env: { MAIL_FROM_OUTREACH: "outreach@civfix.org", MAIL_REPLY_DOMAIN: "civfix.org" }, }) - const loaded: string[] = [] const svc = makeAdminReportService({ repo, outboundMail, now: () => NOW, presignMedia: async (r2Key) => ({ url: `https://media.test/${r2Key}` }), - loadMediaBytes: (r2Key) => { - loaded.push(r2Key) - return Promise.resolve(new Uint8Array(bytesEach)) - }, }) repo.seedReport({ id: "rep-1", @@ -1493,49 +1485,31 @@ describe("F108 report-packet attachments are bounded in AGGREGATE, not just per contact: "311@lacity.gov", routed: false, }, - media: Array.from({ length: count }, (_, i) => ({ - id: `m-${i}`, - kind: "image" as const, - r2Key: `media/photo-${i}.jpg`, - thumbKey: null, - contentType: "image/jpeg", - })), - }) - return { repo, mailer, svc, loaded } + media: [ + { id: "m-0", kind: "image" as const, r2Key: "media/photo-0.jpg", thumbKey: null }, + { id: "m-1", kind: "image" as const, r2Key: "media/photo-1.jpg", thumbKey: null }, + { id: "m-2", kind: "video" as const, r2Key: "media/clip-0.mp4", thumbKey: null }, + ], + }) + return { mailRepo, mailer, svc } } - it("stops attaching once the running total would exceed MAX_PACKET_TOTAL_BYTES", async () => { - const h = harnessWithMedia(4, FOUR_MB) - await h.svc.routeToJurisdiction("rep-1", { - note: null, - actorId: "op-1", - }) - const sent = h.mailer.sent.at(-1)?.outbound - expect(sent?.attachments).toHaveLength(2) - const total = (sent?.attachments ?? []).reduce((n, a) => n + a.content.byteLength, 0) - expect(total).toBeLessThanOrEqual(MAX_PACKET_TOTAL_BYTES) + it("sends no attachments and stores none on the outbound row", async () => { + const h = harnessWithMedia() + const { threadId } = await h.svc.routeToJurisdiction("rep-1", { note: null, actorId: "op-1" }) + expect(h.mailer.sent.at(-1)?.outbound?.attachments).toBeUndefined() + expect(h.mailRepo.messagesOf(threadId).map((m) => m.attachments)).toEqual([[]]) }) - it("still lists every skipped photo as a presigned mediaLink so nothing is lost from the packet", async () => { - const h = harnessWithMedia(4, FOUR_MB) - await h.svc.routeToJurisdiction("rep-1", { - note: null, - actorId: "op-1", - }) - const body = h.mailer.sent.at(-1)?.outbound?.text ?? "" - for (let i = 0; i < 4; i++) { - expect(body).toContain(`https://media.test/media/photo-${i}.jpg`) + it("links every photo and video in the text and html parts", async () => { + const h = harnessWithMedia() + await h.svc.routeToJurisdiction("rep-1", { note: null, actorId: "op-1" }) + const sent = h.mailer.sent.at(-1)?.outbound + for (const key of ["media/photo-0.jpg", "media/photo-1.jpg", "media/clip-0.mp4"]) { + expect(sent?.text).toContain(`https://media.test/${key}`) + expect(sent?.html).toContain(`https://media.test/${key}`) } }) - - it("attaches every photo when the aggregate stays under the cap", async () => { - const h = harnessWithMedia(4, 512 * 1024) - await h.svc.routeToJurisdiction("rep-1", { - note: null, - actorId: "op-1", - }) - expect(h.mailer.sent.at(-1)?.outbound?.attachments).toHaveLength(4) - }) }) describe("runAutoForwardWith delegates the duplicate-send decision", () => { diff --git a/services/api/test/unit/outbound-mail-service.test.ts b/services/api/test/unit/outbound-mail-service.test.ts index cb3074b0..f50a8571 100644 --- a/services/api/test/unit/outbound-mail-service.test.ts +++ b/services/api/test/unit/outbound-mail-service.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest" import { FakeMailer } from "@civfix/shared/fakes" -import type { Mailer, OutboundAttachment, SentMail } from "@civfix/shared/interfaces" +import type { Mailer, SentMail } from "@civfix/shared/interfaces" import { InMemoryMailRepository } from "../../src/services/admin/mail-repository.memory.js" import { assertOutboundSendPolicy, @@ -42,13 +42,8 @@ function harness(): { } describe("OutboundMailService.sendReportToJurisdiction", () => { - function png(): OutboundAttachment { - return { filename: "photo.png", contentType: "image/png", content: new Uint8Array([1, 2, 3, 4]) } - } - - it("sends a per-report packet via sendOutbound From the report- reply address, no Reply-To, + attachments", async () => { + it("sends a per-report packet via sendOutbound From the report- reply address, no Reply-To, no attachments", async () => { const { repo, mailer, svc } = harness() - const attachments = [png()] const { thread, messageId } = await svc.sendReportToJurisdiction({ reportId: "report-1", geoid: "0644000", @@ -57,7 +52,6 @@ describe("OutboundMailService.sendReportToJurisdiction", () => { subject: "civfix report: Pothole [abcd1234]", text: "A pothole on Main St.", html: "

A pothole on Main St.

", - attachments, }) expect(thread.reportId).toBe("report-1") @@ -73,9 +67,7 @@ describe("OutboundMailService.sendReportToJurisdiction", () => { expect(env?.to).toBe("clerk@lacity.gov") expect(env?.subject).toBe("civfix report: Pothole [abcd1234]") expect(env?.html).toBe("

A pothole on Main St.

") - expect(env?.attachments).toHaveLength(1) - expect(env?.attachments?.[0]?.filename).toBe("photo.png") - expect(env?.attachments?.[0]?.contentType).toBe("image/png") + expect(env?.attachments).toBeUndefined() expect(messageId).toMatch(/^$/) expect(env?.messageId).toBe(messageId) @@ -407,7 +399,7 @@ describe("OutboundMailService — F109: a delivered send never throws post-deliv }) describe("OutboundMailService: the outbound row is a true snapshot of what was sent", () => { - it("stores the per-thread From, the html part, the kind and the attachment metadata", async () => { + it("stores the per-thread From, the html part and the kind, with no attachments", async () => { const { repo, svc } = harness() const { thread } = await svc.sendReportToJurisdiction({ reportId: "report-1", @@ -416,17 +408,13 @@ describe("OutboundMailService: the outbound row is a true snapshot of what was s subject: "civfix report: Pothole", text: "A pothole on Main St.", html: "

A pothole on Main St.

", - attachments: [ - { key: "media/r2/photo.jpg", filename: "photo.jpg", contentType: "image/jpeg", content: new Uint8Array([1, 2, 3]) }, - { filename: "keyless.jpg", contentType: "image/jpeg", content: new Uint8Array([4]) }, - ], }) const stored = repo.messagesOf(thread.id)[0] expect(stored?.fromAddr).toBe(`"civfix Reports" `) expect(stored?.html).toBe("

A pothole on Main St.

") expect(stored?.kind).toBe("packet") - expect(stored?.attachments).toEqual([{ key: "media/r2/photo.jpg", filename: "photo.jpg", size: 3 }]) + expect(stored?.attachments).toEqual([]) }) it("stamps the kind each entry point owns", async () => { @@ -838,18 +826,10 @@ describe("OutboundMailService: total send deadline", () => { expect(eightMiB).toBeGreaterThan(oneMiB) }) - it("payload bytes count the text, the html part and attachments AS BASE64 (what goes on the wire)", () => { + it("payload bytes count the text and the html part in UTF-8", () => { expect(outboundPayloadBytes({ body: "abc" })).toBe(3) expect(outboundPayloadBytes({ body: "abc", html: "

de

" })).toBe(3 + 9) - expect( - outboundPayloadBytes({ - body: "abc", - attachments: [ - { filename: "a.jpg", contentType: "image/jpeg", content: new Uint8Array(1000) }, - { filename: "b.jpg", contentType: "image/jpeg", content: new Uint8Array(2000) }, - ], - }), - ).toBe(3 + base64Bytes(3000)) + expect(outboundPayloadBytes({ body: "é" })).toBe(2) expect(base64Bytes(3000)).toBe(4000) })