Skip to content

Send report packets with media links only (#138) #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
theobong merged 1 commit into from
Sep 24, 2026
Merged

Send report packets with media links only (#138) #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions docs/inbound-mail-effects.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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 |
Expand Down
3 changes: 1 addition & 2 deletions services/api/src/adapters/mailer.oci.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
)
Expand Down
2 changes: 0 additions & 2 deletions services/api/src/routes/admin/mail.routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -129,7 +128,6 @@ export async function registerAdminMailRoutes(
return makeMailService({
repo,
outboundMail,
loadAttachmentBytes: (key) => container.storage.getObject(key),
})
},
)
Expand Down
1 change: 0 additions & 1 deletion services/api/src/routes/admin/reports.routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
63 changes: 2 additions & 61 deletions services/api/src/services/admin/admin-report-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -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"]

Expand Down Expand Up @@ -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 =
Expand All @@ -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",
Expand Down Expand Up @@ -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()
}
Expand Down
2 changes: 0 additions & 2 deletions services/api/src/services/admin/admin-report-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export interface AdminReportMediaRecord {
kind: "image" | "video"
r2Key: string
thumbKey: string | null
contentType?: string | null
}

export interface AdminReportTimelineRecord {
Expand Down Expand Up @@ -171,7 +170,6 @@ export interface AdminReportServiceDeps {
loadLinkedEventsForReports?: (
reportIds: string[],
) => Promise<Map<string, LinkedEventView[]>>
loadMediaBytes?: (r2Key: string) => Promise<Uint8Array | null>
now?: () => Date
reportChatEmitter?: ReportChatSystemEmitter
forwardTemplates?: ForwardTemplateReader
Expand Down
1 change: 0 additions & 1 deletion services/api/src/services/admin/autoforward-jobs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Expand Down
26 changes: 0 additions & 26 deletions services/api/src/services/admin/mail-format.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {
"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
Expand Down
5 changes: 1 addition & 4 deletions services/api/src/services/admin/mail-repository.drizzle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import {
type ThreadInit,
} from "./mail-repository.js"
import type {
MailAttachment,
MailDirection,
MailStatsResponse,
MailStatus,
Expand Down Expand Up @@ -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
Expand All @@ -808,7 +806,6 @@ export function makeDrizzleMailRepository(sql: Sql): MailRepository {
subject: row.subject,
body: row.body ?? "",
html: row.html,
attachments: row.attachments ?? [],
}
},

Expand Down
1 change: 0 additions & 1 deletion services/api/src/services/admin/mail-repository.memory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,6 @@ export class InMemoryMailRepository implements MailRepository {
subject: m.subject,
body: m.body ?? "",
html: m.html,
attachments: m.attachments,
})
}

Expand Down
1 change: 0 additions & 1 deletion services/api/src/services/admin/mail-repository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ export interface OutboundMessageSnapshot {
subject: string | null
body: string
html: string | null
attachments: MailAttachment[]
}

export interface RecordEventInput {
Expand Down
43 changes: 1 addition & 42 deletions services/api/src/services/admin/mail-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Uint8Array | null>
}

export interface MailService {
Expand All @@ -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<PacketAttachment[]> {
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<MailThreadDTO> {
const dto = await repo.getThread(id)
if (!dto) throw AppError.notFound("Mail thread not found")
Expand Down Expand Up @@ -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 } : {}),
Comment on lines 135 to 139

@greptile-apps greptile-apps Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Refresh resend media links

Report packets store signed private-media URLs in their text and HTML. This resend path forwards those stored bodies unchanged, so a resend after the seven-day URL lifetime sends links that can no longer retrieve the report photo or video; attachments are no longer included as a fallback. Rebuild report packet links from durable media keys, or retain structured media metadata and mint fresh URLs before delivery. This must be fixed before merging.

Artifacts

Expired packet link reproduction script

  • The focused script creates a private-media report packet and resends it after the modeled link expiry, demonstrating whether the stored URL is refreshed.

Expired packet link reproduction command

  • The command wrapper executes the focused packet generation and resend reproduction.

Original private report packet generation

  • Executed the focused service script to generate the original report packet; it shows one private-media presign from the durable key and the signed URL in the stored text body, establishing the before condition.

Resend after modeled link expiry

  • Executed the focused service script to resend after the seven-day modeled expiry; it shows no refresh call and the expired signed URL remains in the resent body, confirming the defect.

Expired packet link command output

  • The command completed successfully and records that the expired signed URL remained in both resent text and HTML without a refresh.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI 
This is a comment left during a code review.
Path: services/api/src/services/admin/mail-service.ts
Line: 135-139

Comment:
**Refresh resend media links**

Report packets store signed private-media URLs in their text and HTML. This resend path forwards those stored bodies unchanged, so a resend after the seven-day URL lifetime sends links that can no longer retrieve the report photo or video; attachments are no longer included as a fallback. Rebuild report packet links from durable media keys, or retain structured media metadata and mint fresh URLs before delivery. This must be fixed before merging.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

...(attachments.length > 0 ? { attachments } : {}),
kind: "resend",
audit: { actorId, action: "mail.resent", meta: { to: toAddr } },
})
Expand Down
Loading
Loading