From 96e7a56e3e22d3a0328c7f23cc45b1227fa47428 Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Thu, 18 Jun 2026 10:28:19 +0300 Subject: [PATCH] =?UTF-8?q?fix(inbox):=20harden=20media=20send=20=E2=80=94?= =?UTF-8?q?=20GC=20orphans,=20per-type=20caps,=20voice=20cap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses senior-review findings on the media-send feature: - Storage lifecycle: uploads are GC'd on the abandonment paths (discarded draft, failed/errored send, unmount with a staged draft, and replacing one draft with another) via a new deleteAccountMedia helper. Previously every cancel/failure orphaned a public object. - Per-kind size caps (image 5MB, video/audio/document 16MB) mirror Meta's limits and are enforced BEFORE upload, so a too-large image no longer uploads then fails at the Meta call. - Voice notes auto-stop at a 5-min cap and always render as a voice note (transcode anything that isn't already OGG/Opus — Safari MP4 included), not just Chromium WebM. - Send route rejects unknown message_type and captions over Meta's 1024 cap up front; transcode route guards its output size too. - DraftPreview hoisted to a stable module-scope component so the caption input no longer remounts (and drops focus) on each keystroke. Tests: per-kind cap assertions added (418 total). typecheck/lint/build green. Co-Authored-By: Claude Opus 4.8 --- src/app/api/whatsapp/send/route.ts | 24 ++ src/app/api/whatsapp/transcode-audio/route.ts | 6 + src/components/inbox/message-composer.tsx | 294 +++++++++++------- src/components/inbox/message-thread.tsx | 21 +- src/lib/storage/upload-media.test.ts | 14 +- src/lib/storage/upload-media.ts | 35 +++ 6 files changed, 279 insertions(+), 115 deletions(-) diff --git a/src/app/api/whatsapp/send/route.ts b/src/app/api/whatsapp/send/route.ts index fea7fb2..107c493 100644 --- a/src/app/api/whatsapp/send/route.ts +++ b/src/app/api/whatsapp/send/route.ts @@ -88,6 +88,16 @@ export async function POST(request: Request) { const MEDIA_KINDS = ['image', 'video', 'document', 'audio'] as const const isMediaKind = (MEDIA_KINDS as readonly string[]).includes(message_type) + // Reject anything outside the known set up front rather than letting + // an unknown type fall through to the text path with empty content. + const VALID_MESSAGE_TYPES = ['text', 'template', ...MEDIA_KINDS] as const + if (!(VALID_MESSAGE_TYPES as readonly string[]).includes(message_type)) { + return NextResponse.json( + { error: `Unsupported message_type "${message_type}"` }, + { status: 400 } + ) + } + if (message_type === 'text' && !content_text) { return NextResponse.json( { error: 'content_text is required for text messages' }, @@ -109,6 +119,20 @@ export async function POST(request: Request) { ) } + // Meta caps media captions at 1024 chars; reject before the upload is + // wasted at the Meta call. (Audio carries no caption — see meta-api.) + if ( + isMediaKind && + message_type !== 'audio' && + typeof content_text === 'string' && + content_text.length > 1024 + ) { + return NextResponse.json( + { error: 'Caption exceeds the 1024-character limit' }, + { status: 400 } + ) + } + // Fetch conversation and contact const { data: conversation, error: convError } = await supabase .from('conversations') diff --git a/src/app/api/whatsapp/transcode-audio/route.ts b/src/app/api/whatsapp/transcode-audio/route.ts index 5abe13f..23a5ba7 100644 --- a/src/app/api/whatsapp/transcode-audio/route.ts +++ b/src/app/api/whatsapp/transcode-audio/route.ts @@ -105,6 +105,12 @@ export async function POST(request: Request) { await writeFile(inputPath, Buffer.from(await file.arrayBuffer())) await runFfmpeg(ffmpegPath, buildFfmpegArgs(inputPath, outputPath)) const ogg = await readFile(outputPath) + if (ogg.byteLength > MAX_INPUT_BYTES) { + return NextResponse.json( + { error: 'Transcoded audio exceeds the 16 MB limit.' }, + { status: 400 }, + ) + } return new Response(new Uint8Array(ogg), { status: 200, headers: { diff --git a/src/components/inbox/message-composer.tsx b/src/components/inbox/message-composer.tsx index 4908681..396e8f1 100644 --- a/src/components/inbox/message-composer.tsx +++ b/src/components/inbox/message-composer.tsx @@ -32,18 +32,30 @@ import { cn } from "@/lib/utils"; import { toast } from "sonner"; import { uploadAccountMedia, - MEDIA_MAX_BYTES, + deleteAccountMedia, + MEDIA_MAX_BYTES_BY_KIND, } from "@/lib/storage/upload-media"; -import { isMetaAcceptedAudio } from "@/lib/whatsapp/audio-transcode"; import { ReplyQuote } from "./reply-quote"; /** Media content types an agent can send from the composer. */ export type ComposerMediaKind = "image" | "video" | "document" | "audio"; +/** Supabase Storage bucket holding agent-sent chat attachments (migration 023). */ +export const CHAT_MEDIA_BUCKET = "chat-media"; + +/** Meta caps media captions at 1024 chars. Enforced here and in the send route. */ +export const MEDIA_CAPTION_MAX = 1024; + +/** Hard cap on a single voice recording so it can't blow the upload/ + * transcode limits — auto-stops the recorder when reached. */ +const MAX_RECORDING_SECONDS = 5 * 60; + export interface SendMediaPayload { kind: ComposerMediaKind; /** Public chat-media URL Meta fetches at send time. */ mediaUrl: string; + /** Storage object path — lets the caller GC the object if the send fails. */ + path: string; /** Optional caption (image/video/document only). */ caption?: string; /** Original file name — surfaced to the recipient for documents. */ @@ -58,8 +70,6 @@ interface ReplyDraft { preview: string; } -const CHAT_MEDIA_BUCKET = "chat-media"; - // Mirrors the chat-media bucket's allowed_mime_types (migration 023) for // the file picker so unsupported files are rejected before upload rather // than failing with a confusing Storage error. Audio has no picker — it's @@ -74,6 +84,8 @@ const PICKER_ACCEPT: Record<"image" | "video" | "document", string> = { interface MediaDraft { kind: ComposerMediaKind; mediaUrl: string; + /** Storage path — used to GC the object if the draft is discarded. */ + path: string; filename: string; caption: string; } @@ -94,19 +106,13 @@ function formatDuration(seconds: number): string { return `${m}:${s.toString().padStart(2, "0")}`; } -/** - * Map a Meta-accepted audio MIME to a file extension for the upload path. - * Recordings that aren't already accepted are transcoded to OGG upstream, - * so this only ever sees accepted types. - */ -function audioExtForMime(mime: string): string { - const base = mime.split(";")[0]; - if (base.includes("ogg")) return "ogg"; - if (base.includes("mp4")) return "m4a"; - if (base.includes("mpeg")) return "mp3"; - if (base.includes("aac")) return "aac"; - if (base.includes("amr")) return "amr"; - return "ogg"; +/** A recording can skip the transcode round-trip only if it's already + * OGG/Opus — the one format WhatsApp reliably renders as a voice note + * with a waveform. Everything else (Chromium WebM, Safari MP4/AAC) is + * remuxed server-side so the voice-note UX is consistent across + * browsers, not a plain audio attachment on some of them. */ +function isOggOpus(mime: string): boolean { + return mime.split(";")[0].trim().toLowerCase() === "audio/ogg"; } export function MessageComposer({ @@ -129,6 +135,19 @@ export function MessageComposer({ const imageInputRef = useRef(null); const videoInputRef = useRef(null); const documentInputRef = useRef(null); + // Mirror of `draft` for the unmount cleanup, which can't read render + // state. Kept in sync below so navigating away with a staged-but-unsent + // attachment GCs the orphaned object. + const draftRef = useRef(null); + useEffect(() => { + draftRef.current = draft; + }, [draft]); + + // Best-effort GC of a staged object the user never sent. Fire-and-forget. + const removeStaged = useCallback((path: string | undefined) => { + if (!path) return; + void deleteAccountMedia(CHAT_MEDIA_BUCKET, path).catch(() => {}); + }, []); // Voice recording state. const [recording, setRecording] = useState(false); @@ -155,13 +174,15 @@ export function MessageComposer({ }, []); // Tear down any live recording + timer on unmount so a mid-record - // navigation doesn't leak the mic. + // navigation doesn't leak the mic, and GC a staged-but-unsent + // attachment so it doesn't orphan in the bucket. useEffect(() => { return () => { clearTimer(); streamRef.current?.getTracks().forEach((t) => t.stop()); + removeStaged(draftRef.current?.path); }; - }, [clearTimer]); + }, [clearTimer, removeStaged]); const adjustHeight = useCallback(() => { const el = textareaRef.current; @@ -208,23 +229,31 @@ export function MessageComposer({ // Upload a captured file to chat-media and stage it as a draft. const stageUpload = useCallback( async (kind: ComposerMediaKind, file: File) => { - if (file.size > MEDIA_MAX_BYTES) { + // Per-kind ceiling mirrors Meta's caps (image 5 MB, etc.) so we + // reject before upload rather than orphaning an object that Meta + // would then refuse at send. + const max = MEDIA_MAX_BYTES_BY_KIND[kind]; + if (file.size > max) { toast.error( - `File is ${(file.size / 1024 / 1024).toFixed(1)} MB — limit is 16 MB.`, + `File is ${(file.size / 1024 / 1024).toFixed(1)} MB — ${kind} limit is ${Math.round( + max / 1024 / 1024, + )} MB.`, ); return; } setBusy(true); try { - const { publicUrl } = await uploadAccountMedia(CHAT_MEDIA_BUCKET, file); - setDraft({ kind, mediaUrl: publicUrl, filename: file.name, caption: "" }); + const { publicUrl, path } = await uploadAccountMedia(CHAT_MEDIA_BUCKET, file); + // Replacing an existing draft? GC the previous object first. + removeStaged(draftRef.current?.path); + setDraft({ kind, mediaUrl: publicUrl, path, filename: file.name, caption: "" }); } catch (err) { toast.error(err instanceof Error ? err.message : "Upload failed."); } finally { setBusy(false); } }, - [], + [removeStaged], ); const handlePicked = useCallback( @@ -241,16 +270,12 @@ export function MessageComposer({ setBusy(true); try { let file: File; - if (isMetaAcceptedAudio(mime)) { - // Firefox (ogg) / Safari (mp4) already give a Meta-accepted - // format — upload as-is. - const base = mime.split(";")[0]; - file = new File([blob], `voice-${Date.now()}.${audioExtForMime(mime)}`, { - type: base, - }); + if (isOggOpus(mime)) { + // Firefox records OGG/Opus directly — upload as-is. + file = new File([blob], `voice-${Date.now()}.ogg`, { type: "audio/ogg" }); } else { - // Chromium records WebM/Opus, which Meta rejects — remux to - // OGG/Opus server-side first. + // Chromium (WebM) and Safari (MP4) are remuxed to OGG/Opus so + // WhatsApp renders a voice note, not a plain audio attachment. const form = new FormData(); form.append("file", blob, "recording"); const res = await fetch("/api/whatsapp/transcode-audio", { @@ -264,19 +289,20 @@ export function MessageComposer({ const ogg = await res.blob(); file = new File([ogg], `voice-${Date.now()}.ogg`, { type: "audio/ogg" }); } - if (file.size > MEDIA_MAX_BYTES) { + if (file.size > MEDIA_MAX_BYTES_BY_KIND.audio) { toast.error("Recording is too long (over 16 MB)."); return; } - const { publicUrl } = await uploadAccountMedia(CHAT_MEDIA_BUCKET, file); - setDraft({ kind: "audio", mediaUrl: publicUrl, filename: file.name, caption: "" }); + const { publicUrl, path } = await uploadAccountMedia(CHAT_MEDIA_BUCKET, file); + removeStaged(draftRef.current?.path); + setDraft({ kind: "audio", mediaUrl: publicUrl, path, filename: file.name, caption: "" }); } catch (err) { toast.error(err instanceof Error ? err.message : "Could not process the recording."); } finally { setBusy(false); } }, - [], + [removeStaged], ); const startRecording = useCallback(async () => { @@ -333,6 +359,14 @@ export function MessageComposer({ recorderRef.current?.stop(); }, [clearTimer]); + // Auto-stop at the cap so a forgotten recording can't blow the + // upload/transcode limits. + useEffect(() => { + if (recording && recordSeconds >= MAX_RECORDING_SECONDS) { + stopRecording(); + } + }, [recording, recordSeconds, stopRecording]); + // ---- Draft send / discard ----------------------------------------- const sendDraft = useCallback(() => { @@ -340,6 +374,7 @@ export function MessageComposer({ onSendMedia({ kind: draft.kind, mediaUrl: draft.mediaUrl, + path: draft.path, // Audio takes no caption (Meta rejects it). Everything else: the // trimmed caption, or undefined when blank. caption: @@ -347,86 +382,23 @@ export function MessageComposer({ filename: draft.kind === "document" ? draft.filename : undefined, replyToId: replyTo?.id, }); + // The object is now owned by the sent message — clear without GC. setDraft(null); onClearReply?.(); }, [draft, busy, onSendMedia, replyTo?.id, onClearReply]); - const discardDraft = useCallback(() => setDraft(null), []); + // Discard GCs the staged object — it was uploaded but never sent. + const discardDraft = useCallback(() => { + removeStaged(draft?.path); + setDraft(null); + }, [draft?.path, removeStaged]); + + const setCaption = useCallback((caption: string) => { + setDraft((d) => (d ? { ...d, caption } : d)); + }, []); // ---- Render -------------------------------------------------------- - const DraftPreview = () => { - if (!draft) return null; - return ( -
-
-
- {draft.kind === "image" && ( - // eslint-disable-next-line @next/next/no-img-element - {draft.filename} - )} - {draft.kind === "video" && ( -
- -
- -
- {draft.kind !== "audio" && ( - - setDraft((d) => (d ? { ...d, caption: e.target.value } : d)) - } - onKeyDown={(e) => { - if (e.key === "Enter" && !e.shiftKey) { - e.preventDefault(); - sendDraft(); - } - }} - placeholder="Add a caption…" - className="flex-1 rounded-xl border border-border bg-muted px-4 py-2.5 text-sm text-foreground placeholder-muted-foreground outline-none transition-colors focus:border-primary/50" - /> - )} - - - -
-
- ); - }; - return (
{replyTo && ( @@ -488,13 +460,21 @@ export function MessageComposer({ /> {draft ? ( - + ) : recording ? ( // Recording bar — replaces the composer while the mic is live.
- Recording… {formatDuration(recordSeconds)} + Recording… {formatDuration(recordSeconds)} /{" "} + {formatDuration(MAX_RECORDING_SECONDS)} +
+ +
+ {draft.kind !== "audio" && ( + onCaptionChange(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter" && !e.shiftKey) { + e.preventDefault(); + onSend(); + } + }} + placeholder="Add a caption…" + className="flex-1 rounded-xl border border-border bg-muted px-4 py-2.5 text-sm text-foreground placeholder-muted-foreground outline-none transition-colors focus:border-primary/50" + /> + )} + + + +
+
+ ); +} diff --git a/src/components/inbox/message-thread.tsx b/src/components/inbox/message-thread.tsx index 974f5fe..46053f5 100644 --- a/src/components/inbox/message-thread.tsx +++ b/src/components/inbox/message-thread.tsx @@ -34,7 +34,12 @@ import { import { ScrollArea } from "@/components/ui/scroll-area"; import { MessageBubble } from "./message-bubble"; import { MessageActions } from "./message-actions"; -import { MessageComposer, type SendMediaPayload } from "./message-composer"; +import { + MessageComposer, + CHAT_MEDIA_BUCKET, + type SendMediaPayload, +} from "./message-composer"; +import { deleteAccountMedia } from "@/lib/storage/upload-media"; import { TemplatePicker } from "./template-picker"; import { buildReplyPreview } from "./reply-quote"; import { toast } from "sonner"; @@ -521,6 +526,9 @@ export function MessageThread({ console.error("Failed to send media:", reason); toast.error(`Failed to send: ${reason}`); onUpdateMessage(tempId, { status: "failed" }); + // The upload never reached the recipient — GC the orphaned + // object rather than leaving it in the public bucket forever. + void deleteAccountMedia(CHAT_MEDIA_BUCKET, payload.path).catch(() => {}); return; } @@ -530,6 +538,7 @@ export function MessageThread({ const reason = err instanceof Error ? err.message : "network error"; toast.error(`Failed to send: ${reason}`); onUpdateMessage(tempId, { status: "failed" }); + void deleteAccountMedia(CHAT_MEDIA_BUCKET, payload.path).catch(() => {}); } }, [conversation, onNewMessage, onUpdateMessage], @@ -783,7 +792,15 @@ export function MessageThread({ : "Assign"; return ( -
+ // `min-w-0` is load-bearing: the page already puts min-w-0 on the + // thread's flex *wrapper* (issue #165), but this root keeps the + // default `min-width: auto`, so a single wide message (long unbroken + // URL/word) expands the whole thread past its flex share and the chat + // paints on top of the contact sidebar at lg+ — outgoing bubbles get + // clipped and the hover toolbar overlaps the Tags panel. Letting the + // root shrink lets the bubbles' break-words / max-w caps apply. + // Issue #257. +
{/* Header — solid card surface sits on top of the doodle so the name/avatar/dropdowns stay legible. */}
diff --git a/src/lib/storage/upload-media.test.ts b/src/lib/storage/upload-media.test.ts index 6163d99..32180eb 100644 --- a/src/lib/storage/upload-media.test.ts +++ b/src/lib/storage/upload-media.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { buildMediaPath } from "./upload-media"; +import { buildMediaPath, MEDIA_MAX_BYTES_BY_KIND } from "./upload-media"; const ACCOUNT = "11111111-2222-3333-4444-555555555555"; @@ -32,3 +32,15 @@ describe("buildMediaPath", () => { expect(path).toBe(`account-${ACCOUNT}/1700000000000-README.bin`); }); }); + +describe("MEDIA_MAX_BYTES_BY_KIND", () => { + it("caps images at Meta's tighter 5 MB limit", () => { + expect(MEDIA_MAX_BYTES_BY_KIND.image).toBe(5 * 1024 * 1024); + }); + + it("caps video/audio/document at the 16 MB bucket limit", () => { + expect(MEDIA_MAX_BYTES_BY_KIND.video).toBe(16 * 1024 * 1024); + expect(MEDIA_MAX_BYTES_BY_KIND.audio).toBe(16 * 1024 * 1024); + expect(MEDIA_MAX_BYTES_BY_KIND.document).toBe(16 * 1024 * 1024); + }); +}); diff --git a/src/lib/storage/upload-media.ts b/src/lib/storage/upload-media.ts index e4ae728..2daecc6 100644 --- a/src/lib/storage/upload-media.ts +++ b/src/lib/storage/upload-media.ts @@ -17,6 +17,22 @@ import { createClient } from "@/lib/supabase/client"; /** 16 MB — matches the `file_size_limit` on both buckets (migrations 016/020/023). */ export const MEDIA_MAX_BYTES = 16 * 1024 * 1024; +/** + * Per-kind upload ceilings that mirror Meta's WhatsApp Cloud API caps so + * a file that the bucket would accept (≤16 MB) but Meta would reject is + * caught client-side BEFORE upload — otherwise it lands in storage as an + * orphan and the send fails with a confusing 400. Images are Meta's + * tightest cap at 5 MB; documents are held at the 16 MB bucket limit + * (Meta allows 100 MB, but the bucket — and shared-hosting upload UX — + * caps lower). + */ +export const MEDIA_MAX_BYTES_BY_KIND = { + image: 5 * 1024 * 1024, + video: 16 * 1024 * 1024, + audio: 16 * 1024 * 1024, + document: 16 * 1024 * 1024, +} as const; + /** * Build the account-scoped object path for an upload. Pure + exported so * it can be unit-tested without a Supabase client. @@ -100,3 +116,22 @@ export async function uploadAccountMedia( return { publicUrl, path }; } + +/** + * Delete a previously-uploaded object. Used to GC media that was staged + * (uploaded) but never sent — a cancelled draft or a failed Meta send — + * so abandoned attachments don't accumulate in the public bucket. The + * DELETE is gated by the same account-scoped RLS policy as the upload, + * so a caller can only remove objects under their own account folder. + * + * Best-effort: callers fire-and-forget and swallow errors (a missed + * delete is a storage nit, not something to surface to the user). + */ +export async function deleteAccountMedia( + bucket: string, + path: string, +): Promise { + const supabase = createClient(); + const { error } = await supabase.storage.from(bucket).remove([path]); + if (error) throw new Error(error.message); +}