From 4f789f43da20f3444e266a12b0ccf536fcfcc887 Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Wed, 5 Aug 2026 07:33:16 +0100 Subject: [PATCH 01/11] Fix media attachment actions Signed-off-by: kenny lopez --- .../features/messages/lib/useMediaUpload.ts | 134 ++++++++---------- .../messages/ui/ComposerAttachments.tsx | 78 +++++++--- desktop/tests/e2e/composer-image-draw.spec.ts | 39 ++++- desktop/tests/e2e/file-attachment.spec.ts | 62 +++++++- desktop/tests/e2e/spoiler.spec.ts | 3 +- 5 files changed, 208 insertions(+), 108 deletions(-) diff --git a/desktop/src/features/messages/lib/useMediaUpload.ts b/desktop/src/features/messages/lib/useMediaUpload.ts index b4c3cae44f..bdf3fa5892 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.ts +++ b/desktop/src/features/messages/lib/useMediaUpload.ts @@ -136,7 +136,7 @@ async function captureVideoPosterFrame( } type UseMediaUploadOptions = { - /** Keep newly selected files local until the message is submitted. */ + /** Keep newly selected videos local until the message is submitted. */ deferUploadsUntilSend?: boolean; }; @@ -151,6 +151,10 @@ export function useMediaUpload({ const queueUntilSend = deferUploadsUntilSend && (!e2eConfig || e2eConfig.mock?.deferredComposerUploads === true); + const shouldQueueFile = React.useCallback( + (file: File) => queueUntilSend && file.type.startsWith("video/"), + [queueUntilSend], + ); const [uploadState, setUploadState] = React.useState({ status: "idle", }); @@ -481,6 +485,43 @@ export function useMediaUpload({ [finishUpload, isUploadCanceled], ); + const uploadFiles = React.useCallback( + (files: File[]) => { + if (files.length === 0) return; + + setUploadingCount((count) => count + files.length); + const baseIndex = reserveSlots(files.length); + + for (let index = 0; index < files.length; index++) { + const file = files[index]; + const slotIndex = baseIndex + index; + const previewId = reserveUploadingPreview(file, slotIndex); + // Fire-and-forget each upload concurrently — slot preserves order. + void (async () => { + try { + const buffer = await file.arrayBuffer(); + if (isUploadCanceled(previewId)) return; + const descriptor = await uploadMediaBytes( + [...new Uint8Array(buffer)], + file.name, + uploadProgressId(previewId), + ); + fillSlot(slotIndex, descriptor, previewId); + } catch (err) { + onUploadError(err, previewId); + } + })(); + } + }, + [ + fillSlot, + isUploadCanceled, + onUploadError, + reserveSlots, + reserveUploadingPreview, + ], + ); + const handlePaperclip = React.useCallback(async () => { if (queueUntilSend) { const input = document.createElement("input"); @@ -488,7 +529,11 @@ export function useMediaUpload({ input.multiple = true; input.addEventListener( "change", - () => queueFiles(Array.from(input.files ?? [])), + () => { + const files = Array.from(input.files ?? []); + queueFiles(files.filter(shouldQueueFile)); + uploadFiles(files.filter((file) => !shouldQueueFile(file))); + }, { once: true }, ); input.click(); @@ -520,6 +565,8 @@ export function useMediaUpload({ onUploadError, queueFiles, reserveUploadingPreview, + shouldQueueFile, + uploadFiles, ]); const handleDrop = React.useCallback( @@ -534,44 +581,10 @@ export function useMediaUpload({ // (active-content + executables) and size caps; everything else uploads. const validFiles = files; - if (queueUntilSend) { - queueFiles(validFiles); - return; - } - - setUploadingCount((c) => c + validFiles.length); - const baseIndex = reserveSlots(validFiles.length); - - for (let i = 0; i < validFiles.length; i++) { - const file = validFiles[i]; - const slotIndex = baseIndex + i; - const previewId = reserveUploadingPreview(file, slotIndex); - // Fire-and-forget each upload concurrently — slot preserves order - (async () => { - try { - const buffer = await file.arrayBuffer(); - if (isUploadCanceled(previewId)) return; - const descriptor = await uploadMediaBytes( - [...new Uint8Array(buffer)], - file.name, - uploadProgressId(previewId), - ); - fillSlot(slotIndex, descriptor, previewId); - } catch (err) { - onUploadError(err, previewId); - } - })(); - } + queueFiles(validFiles.filter(shouldQueueFile)); + uploadFiles(validFiles.filter((file) => !shouldQueueFile(file))); }, - [ - reserveSlots, - queueUntilSend, - fillSlot, - isUploadCanceled, - onUploadError, - queueFiles, - reserveUploadingPreview, - ], + [queueFiles, shouldQueueFile, uploadFiles], ); const handleDragEnter = React.useCallback( @@ -639,49 +652,16 @@ export function useMediaUpload({ event.preventDefault(); - if (queueUntilSend) { - queueFiles(mediaFiles); - return; - } - - setUploadingCount((c) => c + mediaFiles.length); - const baseIndex = reserveSlots(mediaFiles.length); - - for (let i = 0; i < mediaFiles.length; i++) { - const file = mediaFiles[i]; - const slotIndex = baseIndex + i; - const previewId = reserveUploadingPreview(file, slotIndex); - (async () => { - try { - const buffer = await file.arrayBuffer(); - if (isUploadCanceled(previewId)) return; - const descriptor = await uploadMediaBytes( - [...new Uint8Array(buffer)], - file.name, - uploadProgressId(previewId), - ); - fillSlot(slotIndex, descriptor, previewId); - } catch (err) { - onUploadError(err, previewId); - } - })(); - } + queueFiles(mediaFiles.filter(shouldQueueFile)); + uploadFiles(mediaFiles.filter((file) => !shouldQueueFile(file))); }, - [ - reserveSlots, - queueUntilSend, - fillSlot, - isUploadCanceled, - onUploadError, - queueFiles, - reserveUploadingPreview, - ], + [queueFiles, shouldQueueFile, uploadFiles], ); /** Upload a File directly — used by Tiptap's editorProps.handlePaste. */ const uploadFile = React.useCallback( async (file: File) => { - if (queueUntilSend) { + if (shouldQueueFile(file)) { queueFiles([file]); return; } @@ -701,12 +681,12 @@ export function useMediaUpload({ } }, [ - queueUntilSend, isUploadCanceled, onUploaded, onUploadError, queueFiles, reserveUploadingPreview, + shouldQueueFile, ], ); diff --git a/desktop/src/features/messages/ui/ComposerAttachments.tsx b/desktop/src/features/messages/ui/ComposerAttachments.tsx index 8578e4c083..b5f89b8948 100644 --- a/desktop/src/features/messages/ui/ComposerAttachments.tsx +++ b/desktop/src/features/messages/ui/ComposerAttachments.tsx @@ -5,6 +5,7 @@ import { Bot, FileText, HatGlasses, + LineSquiggle, Pencil, Play, UploadCloud, @@ -36,6 +37,9 @@ import { Toggle } from "@/shared/ui/toggle"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; import { ComposerImageEditor } from "./ComposerImageEditor"; +const COMPOSER_MEDIA_HOVER_ACTION_CLASS = + "absolute inset-0 z-[1] hidden items-center justify-center rounded-2xl bg-black/35 text-white backdrop-blur-[1px] hover:bg-black/45 group-hover:flex"; + /** Dashed-border overlay shown when a file is dragged over the composer form. */ export function DropZoneOverlay({ className }: { className?: string }) { return ( @@ -63,7 +67,7 @@ type ComposerAttachmentsProps = { onCancelUpload?: (previewId: number) => void; /** Remove a local attachment that has not started uploading yet. */ onRemoveQueued?: (previewId: number) => void; - /** Toggle spoiler state for a local attachment before it receives a URL. */ + /** Toggle spoiler state for a queued video before it receives a URL. */ onToggleQueuedSpoiler?: (previewId: number) => void; /** Local previews that are queued for upload when the message is sent. */ queuedPreviews?: UploadingAttachmentPreview[]; @@ -293,9 +297,13 @@ const MediaAttachmentItem = React.forwardRef< const handleRevert = React.useCallback(() => { onRevert?.(attachment.url); }, [attachment.url, onRevert]); + const handleOpenLightbox = React.useCallback(() => { + setOpen(true); + }, []); return ( onRemove(attachment.url)} - className="absolute -right-1 -top-1 hidden h-4 w-4 items-center justify-center rounded-full bg-foreground text-background group-hover:flex" + className="absolute -right-1 -top-1 z-10 hidden h-4 w-4 items-center justify-center rounded-full bg-foreground text-background group-hover:flex" > Remove attachment + {canEdit ? ( + + + + + Draw on image + + ) : null} + {isVideo && onToggleSpoiler ? ( + + + + + + {isSpoilered ? "Remove spoiler" : "Mark as spoiler"} + + + ) : null} ); @@ -620,13 +657,13 @@ export const ComposerAttachments = React.memo(function ComposerAttachments({ ); })} {queuedPreviews.map((preview) => { - const isMedia = - preview.type?.startsWith("image/") || - preview.type?.startsWith("video/"); + const isVideo = preview.type?.startsWith("video/") ?? false; + const isMedia = preview.type?.startsWith("image/") || isVideo; return ( {preview.spoilered ? "Remove spoiler" : "Mark as spoiler"} diff --git a/desktop/tests/e2e/composer-image-draw.spec.ts b/desktop/tests/e2e/composer-image-draw.spec.ts index ea0ce16c88..1ec989079f 100644 --- a/desktop/tests/e2e/composer-image-draw.spec.ts +++ b/desktop/tests/e2e/composer-image-draw.spec.ts @@ -1,11 +1,13 @@ import { expect, type Page, test } from "@playwright/test"; +import { waitForAnimations } from "../helpers/animations"; import { installMockBridge } from "../helpers/bridge"; const ORIGINAL_SHA = "a".repeat(64); const EDITED_SHA = "b".repeat(64); const ORIGINAL_URL = "https://example.com/e2e/draw-original.svg"; const EDITED_URL = "https://example.com/e2e/draw-edited.svg"; +const PR_SNAPSHOT_DIR = "test-results/video-upload-photo-scope"; const ORIGINAL_DESCRIPTOR = { url: ORIGINAL_URL, @@ -67,6 +69,31 @@ test.beforeEach(async ({ page }) => { }); }); +test("image annotation overlay and editor controls", async ({ page }) => { + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await page.getByRole("button", { name: "Attach image" }).click(); + + const composer = page.getByTestId("message-composer"); + await expect(composer.getByAltText("Attachment aaaa")).toBeVisible(); + await composer.getByTestId("composer-media-attachment").hover(); + await expect(page.getByTestId("composer-attachment-annotate")).toBeVisible(); + await waitForAnimations(page); + await composer.screenshot({ + path: `${PR_SNAPSHOT_DIR}/01-image-annotation-overlay.png`, + }); + + await page.getByTestId("composer-attachment-annotate").click(); + const dialog = page.getByRole("dialog"); + await expect(dialog).toBeVisible(); + await expect(page.getByTestId("composer-attachment-edit")).toBeVisible(); + await expect(page.getByTestId("composer-attachment-spoiler")).toBeVisible(); + await waitForAnimations(page); + await dialog.screenshot({ + path: `${PR_SNAPSHOT_DIR}/02-image-editor-controls.png`, + }); +}); + test("draw on an uploaded image, save replaces it, revert restores in place", async ({ page, }) => { @@ -80,7 +107,8 @@ test("draw on an uploaded image, save replaces it, revert restores in place", as await expect(composer.getByAltText("Attachment aaaa")).toBeVisible(); // Open the composer lightbox. - await composer.getByAltText("Attachment aaaa").click(); + await composer.getByTestId("composer-media-attachment").hover(); + await page.getByTestId("composer-attachment-annotate").click(); const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); await expect(dialog.locator(`img[src="${ORIGINAL_URL}"]`)).toBeVisible(); @@ -132,7 +160,8 @@ test("draw on an uploaded image, save replaces it, revert restores in place", as expect(uploadCommandCount).toBe(1); // Reopen the lightbox on the annotated attachment to revert. - await composer.getByAltText("Attachment bbbb").click(); + await composer.getByTestId("composer-media-attachment").hover(); + await page.getByTestId("composer-attachment-annotate").click(); await expect(dialog).toBeVisible(); await expect(dialog.locator(`img[src="${EDITED_URL}"]`)).toBeVisible(); @@ -160,12 +189,14 @@ test("spoiler marking survives drawing on the attachment", async ({ page }) => { // Spoiler the attachment from its lightbox (media spoilers are // per-attachment; the text spoiler control no longer affects media), // then draw on it. - await composer.getByAltText("Attachment aaaa").click(); + await composer.getByTestId("composer-media-attachment").hover(); + await page.getByTestId("composer-attachment-annotate").click(); await page.getByTestId("composer-attachment-spoiler").click(); await page.keyboard.press("Escape"); await expect(composer.locator("[data-composer-media-spoiler]")).toBeVisible(); - await composer.getByAltText("Attachment aaaa").click(); + await composer.getByTestId("composer-media-attachment").hover(); + await page.getByTestId("composer-attachment-annotate").click(); await page.getByTestId("composer-attachment-edit").click(); await drawStrokeOnCanvas(page); diff --git a/desktop/tests/e2e/file-attachment.spec.ts b/desktop/tests/e2e/file-attachment.spec.ts index 699e711984..cf5f3ef1b9 100644 --- a/desktop/tests/e2e/file-attachment.spec.ts +++ b/desktop/tests/e2e/file-attachment.spec.ts @@ -51,12 +51,59 @@ async function chooseLargeVideo(page: Page) { }); } +async function choosePhoto(page: Page) { + const [chooser] = await Promise.all([ + page.waitForEvent("filechooser"), + page.getByRole("button", { name: "Attach image" }).click(), + ]); + await chooser.setFiles({ + buffer: Buffer.from("photo"), + mimeType: "image/png", + name: "photo.png", + }); +} + +test("photos upload before Send without a queued spoiler control", async ({ + page, +}) => { + await page.goto("/"); + await page.evaluate(() => { + const e2e = ( + window as Window & { + __BUZZ_E2E__?: { mock?: { uploadDelayMs?: number } }; + } + ).__BUZZ_E2E__; + if (e2e?.mock) e2e.mock.uploadDelayMs = 1_000; + }); + await page.getByTestId("channel-general").click(); + await choosePhoto(page); + + await expect(page.getByTestId("upload-progress")).toBeVisible(); + await expect(page.getByTestId("composer-queued-video-spoiler")).toHaveCount( + 0, + ); + await expect(page.getByTestId("upload-progress")).toHaveCount(0, { + timeout: 5_000, + }); + await expect(page.getByTestId("composer-upload-progress")).toHaveCount(0); + + await expect + .poll(() => + page.evaluate( + () => + (window as Window & { __BUZZ_E2E_COMMANDS__?: string[] }) + .__BUZZ_E2E_COMMANDS__ ?? [], + ), + ) + .toContain("upload_media_bytes"); +}); + test("upload a file and see a FileCard in the timeline", async ({ page }) => { await page.goto("/"); await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); - // The paperclip queues the local file without starting its upload. + // Non-video files keep the established immediate-upload behavior. await chooseQuarterlyReport(page); // The composer shows a chip with the original filename. @@ -103,13 +150,18 @@ test("sends immediately and keeps upload progress across channels", async ({ if (e2e?.mock) e2e.mock.uploadDelayMs = 1_000; }); await page.getByTestId("channel-general").click(); - await chooseQuarterlyReport(page); + await chooseLargeVideo(page); await expect(page.getByTestId("composer-upload-progress")).toHaveCount(0); + await expect(page.getByTestId("composer-video-spoiler")).toHaveCount(0); + const queuedSpoiler = page.getByTestId("composer-queued-video-spoiler"); + await expect(queuedSpoiler).toBeHidden(); + await page.getByTestId("composer-queued-media-attachment").hover(); + await expect(queuedSpoiler).toBeVisible(); await page.getByTestId("send-message").click(); await expect(page.getByTestId("message-composer")).not.toContainText( - "quarterly-report.pdf", + "large-video.mp4", ); await expect(page.getByTestId("composer-upload-progress")).toBeVisible(); @@ -226,7 +278,7 @@ test("canceling a background upload prevents the message from publishing", async if (e2e?.mock) e2e.mock.uploadDelayMs = 1_000; }); await page.getByTestId("channel-general").click(); - await chooseQuarterlyReport(page); + await chooseLargeVideo(page); await page.getByTestId("send-message").click(); await page.getByTestId("composer-upload-cancel").click(); @@ -259,7 +311,7 @@ test("upload progress floats above the dock and lifts Jump to latest", async ({ await expect(jumpToLatest).toBeVisible(); const restingBox = await jumpToLatest.boundingBox(); - await chooseQuarterlyReport(page); + await chooseLargeVideo(page); await page.getByTestId("send-message").click(); const uploadMotion = page.getByTestId("composer-upload-progress-motion"); await expect(uploadMotion).toBeVisible(); diff --git a/desktop/tests/e2e/spoiler.spec.ts b/desktop/tests/e2e/spoiler.spec.ts index 738b19a74c..4cf4ac6715 100644 --- a/desktop/tests/e2e/spoiler.spec.ts +++ b/desktop/tests/e2e/spoiler.spec.ts @@ -102,7 +102,8 @@ test("image attachments can be marked and sent as hidden spoilers", async ({ await expect(composer.getByAltText("Attachment cccc")).toBeVisible(); // Media spoilers are toggled per-attachment from the lightbox. - await composer.getByAltText("Attachment cccc").click(); + await composer.getByTestId("composer-media-attachment").hover(); + await page.getByTestId("composer-attachment-annotate").click(); await page.getByTestId("composer-attachment-spoiler").click(); await page.keyboard.press("Escape"); await expect(composer.locator("[data-composer-media-spoiler]")).toBeVisible(); From d90da2890e8fecfb87f3a095be7e4fee9308cc16 Mon Sep 17 00:00:00 2001 From: Honey <47c18026466e670a1618fd7de0ef32b9ff75d6e0b5ccf255d13c8c3d674ed115@buzz.block.builderlab.xyz> Date: Wed, 5 Aug 2026 08:16:01 +0100 Subject: [PATCH 02/11] Block send while an immediate attachment upload is in flight Photos and generic files now upload as soon as they are attached, so an in-flight upload lives in neither pendingImeta nor queuedAttachments. The normal send path did not gate on media.isUploading, so a draft that already had text could be sent mid-upload: the message published without the attachment and the descriptor landed in an already-cleared composer. Gate both submitMessage and the Send button on media.isUploading (the edit-only condition is now unconditional) and assert the disabled/enabled transition in the photo-upload e2e test. Co-authored-by: kenny lopez Signed-off-by: kenny lopez --- desktop/src/features/messages/lib/useMediaUpload.ts | 9 +++++++++ desktop/src/features/messages/ui/MessageComposer.tsx | 4 ++-- desktop/tests/e2e/file-attachment.spec.ts | 5 +++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/desktop/src/features/messages/lib/useMediaUpload.ts b/desktop/src/features/messages/lib/useMediaUpload.ts index bdf3fa5892..5f1eab00b9 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.ts +++ b/desktop/src/features/messages/lib/useMediaUpload.ts @@ -781,6 +781,15 @@ export function useMediaUpload({ [], ); + /** + * True while any attachment upload is in flight. + * + * Send paths must gate on this: with `deferUploadsUntilSend`, only videos + * are queued locally, so an in-flight photo/file is in neither + * `pendingImeta` nor `queuedAttachments`. Sending mid-flight would publish + * the message without that attachment and land the descriptor in an + * already-cleared composer. + */ const isUploading = uploadingCount > 0; const queuedPreviews = React.useMemo( () => diff --git a/desktop/src/features/messages/ui/MessageComposer.tsx b/desktop/src/features/messages/ui/MessageComposer.tsx index 6f79daa606..3432270f0f 100644 --- a/desktop/src/features/messages/ui/MessageComposer.tsx +++ b/desktop/src/features/messages/ui/MessageComposer.tsx @@ -555,6 +555,7 @@ function MessageComposerImpl({ (!trimmed && !hasMedia) || disabledRef.current || isSendingRef.current || + isUploadingRef.current || mentionSendFlow.isPreparingMentionSend ) { return; @@ -804,14 +805,13 @@ function MessageComposerImpl({ const sendDisabled = React.useMemo( () => composerDisabled || - (editTarget !== null && media.isUploading) || + media.isUploading || mentionSendFlow.isPreparingMentionSend || (isContentEmpty && media.pendingImeta.length === 0 && media.queuedAttachments.length === 0), [ composerDisabled, - editTarget, media.isUploading, mentionSendFlow.isPreparingMentionSend, isContentEmpty, diff --git a/desktop/tests/e2e/file-attachment.spec.ts b/desktop/tests/e2e/file-attachment.spec.ts index cf5f3ef1b9..67c65531e1 100644 --- a/desktop/tests/e2e/file-attachment.spec.ts +++ b/desktop/tests/e2e/file-attachment.spec.ts @@ -82,10 +82,15 @@ test("photos upload before Send without a queued spoiler control", async ({ await expect(page.getByTestId("composer-queued-video-spoiler")).toHaveCount( 0, ); + // Photos upload immediately, so they are in neither `pendingImeta` nor the + // queued list until the upload lands: Send stays blocked so the message + // cannot publish without the attachment. + await expect(page.getByTestId("send-message")).toBeDisabled(); await expect(page.getByTestId("upload-progress")).toHaveCount(0, { timeout: 5_000, }); await expect(page.getByTestId("composer-upload-progress")).toHaveCount(0); + await expect(page.getByTestId("send-message")).toBeEnabled(); await expect .poll(() => From ed8e7fdfdb8c7029321b62601efab41dc9bbe48a Mon Sep 17 00:00:00 2001 From: Honey <47c18026466e670a1618fd7de0ef32b9ff75d6e0b5ccf255d13c8c3d674ed115@buzz.block.builderlab.xyz> Date: Wed, 5 Aug 2026 08:40:36 +0100 Subject: [PATCH 03/11] Discard immediate uploads that outlive their originating draft The main mediaController is reused across channel/draft changes, and the draft-key lifecycle only clears and restores pendingImeta and queued attachments. Since photos and generic files now upload immediately, an upload can still be in flight when the composer swaps drafts: the captured fillSlot then wrote into the now-current imetaSlots, so the previous channel's attachment could appear in -- or overwrite a slot reserved by -- the new draft. Track an upload epoch that bumps on every wholesale setPendingImeta replacement (draft/channel switch, post-send clear, edit restore). Uploads pin the epoch at start; fillSlot, onUploaded, and the native paperclip path drop their descriptor when it no longer matches, while still settling the uploading count so progress UI does not leak. The updater form of setPendingImeta appends within the current draft, so it does not bump. uploadEditedAttachment needs no epoch: it swaps by URL and no-ops when the target is no longer pending. Co-authored-by: kenny lopez Signed-off-by: kenny lopez --- .../messages/lib/useMediaUpload.test.mjs | 54 +++++++++++++++++ .../features/messages/lib/useMediaUpload.ts | 60 +++++++++++++++++-- 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/lib/useMediaUpload.test.mjs b/desktop/src/features/messages/lib/useMediaUpload.test.mjs index 674cce5ffd..8ed7b30824 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.test.mjs +++ b/desktop/src/features/messages/lib/useMediaUpload.test.mjs @@ -142,3 +142,57 @@ test("reserveSlots pads if slots array is shorter than expected start index", () assert.equal(next[3], null); // reserved assert.equal(next[4], null); // reserved }); + +// ── Draft-boundary epoch guard (pure logic) ─────────────────────────── +// Photos/files upload immediately, so an upload can still be in flight when +// the composer swaps drafts (channel switch, post-send clear, edit restore). +// Every wholesale `setPendingImeta` replacement bumps an epoch; uploads pin +// the epoch at start and discard their descriptor if it no longer matches, so +// one draft's attachment can never land in — or overwrite a slot reserved by — +// another draft. Mirrors `isUploadStale` + `fillSlot`/`onUploaded`. + +function fillSlotIfCurrent(slots, index, descriptor, epoch, currentEpoch) { + if (epoch !== currentEpoch) return slots; + const next = [...slots]; + next[index] = descriptor; + return next; +} + +test("upload completing in the same draft fills its slot", () => { + const a = { url: "a.png", sha256: "aaaa" }; + const next = fillSlotIfCurrent([null], 0, a, 0, 0); + assert.deepEqual(next, [a]); +}); + +test("upload completing after a draft switch is discarded", () => { + // Draft A reserves slot 0 at epoch 0, user switches channels (epoch → 1), + // then the upload resolves. It must not write into draft B's slots. + const a = { url: "a.png", sha256: "aaaa" }; + const draftBSlots = [null]; + const next = fillSlotIfCurrent(draftBSlots, 0, a, 0, 1); + assert.deepEqual(next, [null]); + assert.equal(next, draftBSlots); +}); + +test("stale upload cannot overwrite a slot the new draft already filled", () => { + // Draft B has its own attachment in slot 0; draft A's late upload targets + // the same index and must leave B's descriptor intact. + const stale = { url: "stale.png", sha256: "aaaa" }; + const current = { url: "current.png", sha256: "bbbb" }; + const next = fillSlotIfCurrent([current], 0, stale, 0, 2); + assert.deepEqual(next, [current]); +}); + +test("appending to the current draft does not bump the epoch", () => { + // Only wholesale replacement (`setPendingImeta(array)`) is a draft boundary. + // The updater form appends within the current draft, so in-flight uploads + // for that same draft must still be considered current. + let epoch = 0; + const bumpIfReplacement = (action) => { + if (typeof action !== "function") epoch += 1; + }; + bumpIfReplacement((current) => [...current, { url: "pasted.png" }]); + assert.equal(epoch, 0); + bumpIfReplacement([]); + assert.equal(epoch, 1); +}); diff --git a/desktop/src/features/messages/lib/useMediaUpload.ts b/desktop/src/features/messages/lib/useMediaUpload.ts index 5f1eab00b9..cd3c28a150 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.ts +++ b/desktop/src/features/messages/lib/useMediaUpload.ts @@ -208,6 +208,13 @@ export function useMediaUpload({ }, []); const activeUploadingPreviewIdsRef = React.useRef(new Set()); const canceledUploadingPreviewIdsRef = React.useRef(new Set()); + /** + * Incremented whenever the composer's attachment set is replaced wholesale + * (draft/channel switch, post-send clear, edit restore). Uploads capture the + * epoch at start and discard their result if it no longer matches, so an + * upload started against one draft can never land in another. + */ + const uploadEpochRef = React.useRef(0); // ── Drag-over visual indicator state ─────────────────────────────── const [isDragOver, setIsDragOver] = React.useState(false); @@ -451,10 +458,29 @@ export function useMediaUpload({ return startIndex; }, []); + /** + * True when the composer's attachment set was replaced since `epoch` was + * captured, meaning an upload that started then belongs to a draft that is + * no longer on screen and must not write its descriptor. + */ + const isUploadStale = React.useCallback( + (epoch: number) => epoch !== uploadEpochRef.current, + [], + ); + /** Fill a previously-reserved slot by index. */ const fillSlot = React.useCallback( - (index: number, descriptor: BlobDescriptor, previewId?: number) => { + ( + index: number, + descriptor: BlobDescriptor, + previewId?: number, + epoch = uploadEpochRef.current, + ) => { if (isUploadCanceled(previewId)) return; + if (isUploadStale(epoch)) { + finishUpload(previewId); + return; + } setImetaSlots((prev) => { const next = [...prev]; next[index] = descriptor; @@ -462,18 +488,26 @@ export function useMediaUpload({ }); finishUpload(previewId); }, - [finishUpload, isUploadCanceled], + [finishUpload, isUploadCanceled, isUploadStale], ); /** Append a single descriptor (no pre-reserved slot). */ const onUploaded = React.useCallback( - (descriptor: BlobDescriptor, previewId?: number) => { + ( + descriptor: BlobDescriptor, + previewId?: number, + epoch = uploadEpochRef.current, + ) => { if (isUploadCanceled(previewId)) return; + if (isUploadStale(epoch)) { + finishUpload(previewId); + return; + } nextSlotRef.current += 1; setImetaSlots((prev) => [...prev, descriptor]); finishUpload(previewId); }, - [finishUpload, isUploadCanceled], + [finishUpload, isUploadCanceled, isUploadStale], ); const onUploadError = React.useCallback( @@ -491,6 +525,9 @@ export function useMediaUpload({ setUploadingCount((count) => count + files.length); const baseIndex = reserveSlots(files.length); + // Pin the epoch at start: if the composer swaps drafts mid-flight, these + // completions are discarded rather than written into the new draft. + const epoch = uploadEpochRef.current; for (let index = 0; index < files.length; index++) { const file = files[index]; @@ -506,7 +543,7 @@ export function useMediaUpload({ file.name, uploadProgressId(previewId), ); - fillSlot(slotIndex, descriptor, previewId); + fillSlot(slotIndex, descriptor, previewId, epoch); } catch (err) { onUploadError(err, previewId); } @@ -546,10 +583,12 @@ export function useMediaUpload({ // descriptor when we get them back. const previewId = reserveUploadingPreview(); setUploadingCount((c) => c + 1); + const epoch = uploadEpochRef.current; try { const descriptors = await pickAndUploadMedia(uploadProgressId(previewId)); if (isUploadCanceled(previewId)) return; finishUpload(previewId); + if (isUploadStale(epoch)) return; for (const descriptor of descriptors) { nextSlotRef.current += 1; setImetaSlots((prev) => [...prev, descriptor]); @@ -562,6 +601,7 @@ export function useMediaUpload({ queueUntilSend, finishUpload, isUploadCanceled, + isUploadStale, onUploadError, queueFiles, reserveUploadingPreview, @@ -667,6 +707,7 @@ export function useMediaUpload({ } const previewId = reserveUploadingPreview(file); setUploadingCount((c) => c + 1); + const epoch = uploadEpochRef.current; try { const buffer = await file.arrayBuffer(); if (isUploadCanceled(previewId)) return; @@ -675,7 +716,7 @@ export function useMediaUpload({ file.name, uploadProgressId(previewId), ); - onUploaded(descriptor, previewId); + onUploaded(descriptor, previewId, epoch); } catch (err) { onUploadError(err, previewId); } @@ -771,6 +812,13 @@ export function useMediaUpload({ /** Public setter — replaces all slots (used by MessageComposer to clear/restore). */ const setPendingImeta = React.useCallback( (action: React.SetStateAction) => { + // A wholesale replacement means the composer's contents were swapped out + // from under any in-flight upload: draft/channel switch, post-send clear, + // or edit-target restore. Bump the epoch so those uploads discard their + // results instead of landing in (or overwriting a slot reserved by) the + // draft that is now on screen. The updater form is an append against the + // *current* draft (e.g. agent-snapshot paste), so it must NOT bump. + if (typeof action !== "function") uploadEpochRef.current += 1; setImetaSlots((prev) => { const current = prev.filter((d): d is BlobDescriptor => d !== null); const next = typeof action === "function" ? action(current) : action; From 1e4e250839908d4ae84c7fa835d9a59d644eb666 Mon Sep 17 00:00:00 2001 From: Honey <47c18026466e670a1618fd7de0ef32b9ff75d6e0b5ccf255d13c8c3d674ed115@buzz.block.builderlab.xyz> Date: Wed, 5 Aug 2026 10:13:46 +0100 Subject: [PATCH 04/11] Guard stale upload previews and queue videos with no MIME type Two follow-on correctness issues from the immediate-upload split, neither of which changes the intended UI or behavior. Stale previews could hijack the wrong draft. The upload epoch made late completions discard their descriptors, but the old preview row and its cancel button stayed on screen. Cancelling one ran cancelUpload with the replaced draft's slotIndex, nulling whatever attachment now occupied that slot in the draft on screen. Previews now record the epoch they were created in and cancel skips the slot-nulling when it no longer matches. Videos with a missing or opaque MIME type were not queued. shouldQueueFile was MIME-only, so a .mp4 arriving as empty or application/octet-stream (no OS MIME entry, network shares, some pickers) uploaded in the foreground and blocked Send instead of taking the background path. Add a filename-extension fallback in a new videoFileType helper, with a concrete MIME type still authoritative so an image/gif named .mp4 stays an image. The same helper re-types the blob URL used for poster capture, which otherwise yields no poster for those files. Co-authored-by: kenny lopez Signed-off-by: kenny lopez --- .../messages/lib/useMediaUpload.test.mjs | 28 +++++ .../features/messages/lib/useMediaUpload.ts | 53 +++++++-- .../messages/lib/videoFileType.test.mjs | 106 ++++++++++++++++++ .../features/messages/lib/videoFileType.ts | 71 ++++++++++++ 4 files changed, 246 insertions(+), 12 deletions(-) create mode 100644 desktop/src/features/messages/lib/videoFileType.test.mjs create mode 100644 desktop/src/features/messages/lib/videoFileType.ts diff --git a/desktop/src/features/messages/lib/useMediaUpload.test.mjs b/desktop/src/features/messages/lib/useMediaUpload.test.mjs index 8ed7b30824..eb7bbd8359 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.test.mjs +++ b/desktop/src/features/messages/lib/useMediaUpload.test.mjs @@ -196,3 +196,31 @@ test("appending to the current draft does not bump the epoch", () => { bumpIfReplacement([]); assert.equal(epoch, 1); }); + +// ── Cancel guard for stale previews (pure logic) ─────────────────────── +// The epoch bump makes completions discard their descriptors, but the old +// preview row (and its cancel button) can still be on screen. Cancelling it +// must not null a slot in the draft now on screen, because the preview carries +// the *previous* draft's slotIndex. Mirrors `cancelUpload`'s `isStalePreview`. + +function cancelSlotIndex(preview, currentEpoch) { + if (preview?.slotIndex === undefined) return undefined; + const isStale = + preview.uploadEpoch !== undefined && preview.uploadEpoch !== currentEpoch; + return isStale ? undefined : preview.slotIndex; +} + +test("cancelling a preview from the current draft nulls its slot", () => { + assert.equal(cancelSlotIndex({ slotIndex: 1, uploadEpoch: 3 }, 3), 1); +}); + +test("cancelling a stale preview does not null the new draft's slot", () => { + // Draft A reserved slot 0 at epoch 0; draft B now owns slot 0. Cancelling + // A's leftover preview must leave B's attachment intact. + assert.equal(cancelSlotIndex({ slotIndex: 0, uploadEpoch: 0 }, 1), undefined); +}); + +test("cancelling a preview with no slot is a no-op for slots", () => { + // `handlePaperclip`'s native-picker preview has no reserved slot. + assert.equal(cancelSlotIndex({ uploadEpoch: 0 }, 0), undefined); +}); diff --git a/desktop/src/features/messages/lib/useMediaUpload.ts b/desktop/src/features/messages/lib/useMediaUpload.ts index cd3c28a150..e8a0fa25c6 100644 --- a/desktop/src/features/messages/lib/useMediaUpload.ts +++ b/desktop/src/features/messages/lib/useMediaUpload.ts @@ -6,6 +6,7 @@ import { uploadMediaBytes, } from "@/shared/api/tauri"; import type { QueuedMediaAttachment } from "./backgroundMediaUploadStore"; +import { isVideoFile, videoMimeForFile } from "./videoFileType"; /** * First 4 hex chars of the sha256 — used as a short display name. @@ -33,6 +34,12 @@ export type UploadingAttachmentPreview = { slotIndex?: number; spoilered?: boolean; type?: string; + /** + * Upload epoch this preview was created in. Cancel handling compares it + * against the current epoch so a preview left over from a replaced draft + * cannot null a slot belonging to the draft now on screen. + */ + uploadEpoch?: number; }; /** Correlation id for the Rust `media-upload-progress` events. */ @@ -85,9 +92,16 @@ type CapturedVideoPoster = { async function captureVideoPosterFrame( file: File, ): Promise { - if (!file.type.startsWith("video/")) return null; - - const objectUrl = URL.createObjectURL(file); + const videoMime = videoMimeForFile(file); + if (!videoMime) return null; + + // A blob URL inherits the File's own MIME type, so a video whose type is + // empty or `application/octet-stream` would be rejected by the