fix: Bug - Sending files and attachment issues

Normalize attachment MIME types from filenames, hydrate playable media and gallery tiles from disk without redundant peer requests, reset stalled partial downloads, and improve gallery retry/hydration UX across chat and DMs. Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-14 13:30:13 +02:00
parent b13f71d2d3
commit 0078c320a5
14 changed files with 388 additions and 15 deletions
@@ -144,7 +144,9 @@ When the user navigates to a room, the manager watches the route and decides whi

 The decision lives in `shouldAutoRequestWhenWatched()` which calls `isAttachmentMedia()` and checks against `MAX_AUTO_SAVE_SIZE_BYTES`.

-Direct-message routes (`/dm/:conversationId` and `/pm/:conversationId`) are treated as watched attachment containers named `direct-message:<conversationId>`, so image/video metadata announced for the visible conversation is eligible for the same automatic request path as server-room media. Auto-download work fans out with bounded concurrency (`ATTACHMENT_AUTO_DOWNLOAD_CONCURRENCY`, default 3 files at a time per watched room) so multiple pending files can progress in parallel without removing the per-file chunk-ack memory safety invariant.
+Direct-message routes (`/dm/:conversationId` and `/pm/:conversationId`) are treated as watched attachment containers named `direct-message:<conversationId>`, so image/video metadata announced for the visible conversation is eligible for the same automatic request path as server-room media. Auto-download work fans out with bounded concurrency (`ATTACHMENT_AUTO_DOWNLOAD_CONCURRENCY`, default 3 files at a time per watched room) so multiple pending files can progress in parallel without removing the per-file chunk-ack memory safety invariant. Stalled partial downloads with no active pending request are reset automatically before the next auto-download pass.
+
+Incoming and synced attachment metadata is normalized through `attachment-normalize.rules.ts` / `attachment-mime.rules.ts`: generic `application/octet-stream` (or empty) MIME types are inferred from the filename extension so images still group into galleries and small audio/video render as players instead of generic file cards. Display hydration (`needsAttachmentDisplayHydration`) rehydrates blob/file URLs from disk for both inline images and playable media without forcing a fresh peer download when local bytes already exist.

 Browser chat views render audio/video larger than 50 MB with the same generic file interface as other downloads, even after the bytes are available. Attachments with audio/video MIME types that Chromium reports as unsupported also use the generic file interface instead of a broken native player.

@@ -17,6 +17,7 @@ import {
  shouldAutoRequestWhenWatched
 } from '../../domain/logic/attachment.logic';
 import { ATTACHMENT_AUTO_DOWNLOAD_CONCURRENCY, runTasksWithBoundedConcurrency } from '../../domain/logic/attachment-autodownload-concurrency.rules';
+import { shouldResetStalledAttachmentDownload } from '../../domain/logic/attachment-autodownload.rules';
 import type { Attachment, AttachmentMeta } from '../../domain/models/attachment.model';
 import type {
  FileAnnouncePayload,
@@ -379,8 +380,14 @@ export class AttachmentManagerService {
      if (attachment.available)
        continue;

-      if ((attachment.receivedBytes ?? 0) > 0)
+      if (shouldResetStalledAttachmentDownload(
+        attachment,
+        this.transfer.hasPendingRequest(messageId, attachment.id)
+      )) {
+        this.transfer.cancelRequest(messageId, attachment);
+      } else if ((attachment.receivedBytes ?? 0) > 0) {
        continue;
+      }

      if (this.transfer.hasPendingRequest(messageId, attachment.id))
        continue;
@@ -866,4 +866,115 @@ describe('AttachmentTransferService', () => {
    expect(attachment.available).toBe(false);
    expect(service.hasPendingRequest(MESSAGE_ID, FILE_ID)).toBe(false);
  });
+
+  it('normalizes generic octet-stream announces into image metadata for gallery grouping', () => {
+    const service = createService();
+
+    expect(service.handleFileAnnounce({
+      messageId: MESSAGE_ID,
+      fromPeerId: PEER_ID,
+      file: {
+        id: FILE_ID,
+        filename: 'grid.png',
+        size: 512,
+        mime: 'application/octet-stream',
+        isImage: false,
+        uploaderPeerId: PEER_ID
+      }
+    })).toBe(true);
+
+    const attachment = runtimeStore.getAttachmentsForMessage(MESSAGE_ID)[0];
+
+    expect(attachment.mime).toBe('image/png');
+    expect(attachment.isImage).toBe(true);
+  });
+
+  it('hydrates playable media from disk without re-requesting when display url is missing', async () => {
+    persistence.tryRestoreAttachmentFromLocal.mockImplementation(async (attachment) => {
+      attachment.objectUrl = 'file:///appdata/song.mp3';
+      attachment.available = true;
+      return true;
+    });
+
+    const service = createService();
+    const attachment: Attachment = {
+      id: FILE_ID,
+      messageId: MESSAGE_ID,
+      filename: 'song.mp3',
+      size: 1024,
+      mime: 'audio/mpeg',
+      isImage: false,
+      uploaderPeerId: PEER_ID,
+      available: true,
+      savedPath: '/appdata/song.mp3',
+      receivedBytes: 0
+    };
+
+    runtimeStore.setAttachmentsForMessage(MESSAGE_ID, [attachment]);
+
+    await service.requestFromAnyPeer(MESSAGE_ID, attachment);
+
+    expect(persistence.tryRestoreAttachmentFromLocal).toHaveBeenCalled();
+    expect(webrtc.sendToPeer).not.toHaveBeenCalled();
+    expect(attachment.objectUrl).toBe('file:///appdata/song.mp3');
+  });
+
+  it('hydrates small in-memory audio downloads with a native file url when available', async () => {
+    attachmentStorage.getFileUrl.mockResolvedValue('file:///appdata/song.mp3');
+    persistence.saveFileToDisk.mockImplementation(async (attachment) => {
+      attachment.savedPath = '/appdata/song.mp3';
+      return attachment.savedPath;
+    });
+
+    const service = createService();
+    const attachment: Attachment = {
+      id: FILE_ID,
+      messageId: MESSAGE_ID,
+      filename: 'song.mp3',
+      size: 1024,
+      mime: 'audio/mpeg',
+      isImage: false,
+      uploaderPeerId: PEER_ID,
+      available: false,
+      receivedBytes: 0
+    };
+
+    runtimeStore.setAttachmentsForMessage(MESSAGE_ID, [attachment]);
+
+    service.handleFileChunk(chunkPayload(0, 1, [
+      1,
+      2,
+      3
+    ]));
+
+    await vi.waitFor(() => expect(attachment.objectUrl).toBe('file:///appdata/song.mp3'));
+
+    expect(attachment.available).toBe(true);
+    expect(attachment.savedPath).toBe('/appdata/song.mp3');
+  });
+
+  it('does not cancel hydrating gallery retries before attempting local restore', async () => {
+    persistence.tryRestoreAttachmentFromLocal.mockResolvedValue(true);
+
+    const service = createService();
+    const attachment: Attachment = {
+      id: FILE_ID,
+      messageId: MESSAGE_ID,
+      filename: 'grid.png',
+      size: 512,
+      mime: 'image/png',
+      isImage: true,
+      uploaderPeerId: PEER_ID,
+      available: false,
+      savedPath: '/appdata/grid.png',
+      receivedBytes: 0
+    };
+
+    runtimeStore.setAttachmentsForMessage(MESSAGE_ID, [attachment]);
+
+    await service.requestImageFromAnyPeer(MESSAGE_ID, attachment);
+
+    expect(persistence.tryRestoreAttachmentFromLocal).toHaveBeenCalled();
+    expect(webrtc.sendToPeer).not.toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ type: 'file-cancel' }));
+  });
 });
@@ -13,10 +13,13 @@ import { isSharingFromThisDevice, canHostAttachment } from '../../domain/logic/a
 import { selectFileRequestPeer } from '../../domain/logic/attachment-request.rules';
 import {
  canReceiveAttachment,
+  needsAttachmentDisplayHydration,
  shouldCopyLargeUploaderFileToAppData,
  shouldPersistDownloadedAttachment,
  shouldStreamAttachmentReceiveToDisk
 } from '../../domain/logic/attachment.logic';
+import { normalizeAttachmentMeta } from '../../domain/logic/attachment-normalize.rules';
+import { resolveAttachmentMime } from '../../domain/logic/attachment-mime.rules';
 import { shouldServeAttachmentFromDiskPath } from '../../domain/logic/attachment-serve.rules';
 import type { Attachment, AttachmentMeta } from '../../domain/models/attachment.model';
 import {
@@ -141,9 +144,11 @@ export class AttachmentTransferService {
        const alreadyKnown = existing.find((entry) => entry.id === meta.id);

        if (!alreadyKnown) {
-          const attachment: Attachment = { ...meta,
+          const attachment: Attachment = {
+            ...normalizeAttachmentMeta(meta),
            available: false,
-            receivedBytes: 0 };
+            receivedBytes: 0
+          };

          existing.push(attachment);
          newAttachments.push(attachment);
@@ -171,6 +176,16 @@ export class AttachmentTransferService {
    // request makes the sender stream the file twice and corrupts byte accounting.
    this.runtimeStore.setPendingRequestPeers(requestKey, new Set<string>());

+    if (needsAttachmentDisplayHydration(attachment)) {
+      const hydratedLocally = await this.persistence.tryRestoreAttachmentFromLocal(attachment);
+
+      if (hydratedLocally) {
+        this.runtimeStore.deletePendingRequest(requestKey);
+        this.runtimeStore.touch();
+        return;
+      }
+    }
+
    if (!attachment.available) {
      const restoredLocally = await this.persistence.tryRestoreAttachmentFromLocal(attachment);

@@ -230,6 +245,10 @@ export class AttachmentTransferService {
  }

  requestImageFromAnyPeer(messageId: string, attachment: Attachment): Promise<void> {
+    if (needsAttachmentDisplayHydration(attachment)) {
+      return this.requestFromAnyPeer(messageId, attachment);
+    }
+
    if ((attachment.receivedBytes ?? 0) > 0 || this.hasPendingRequest(messageId, attachment.id)) {
      this.cancelRequest(messageId, attachment);
    }
@@ -259,7 +278,7 @@ export class AttachmentTransferService {
        messageId,
        filename: file.name,
        size: file.size,
-        mime: file.type || DEFAULT_ATTACHMENT_MIME_TYPE,
+        mime: resolveAttachmentMime(file.name, file.type || DEFAULT_ATTACHMENT_MIME_TYPE),
        isImage: resolvePublishAttachmentIsImage(file),
        uploaderPeerId,
        filePath: (file as LocalFileWithPath).path,
@@ -327,29 +346,40 @@ export class AttachmentTransferService {
    const alreadyKnown = list.find((entry) => entry.id === file.id);

    if (alreadyKnown) {
+      alreadyKnown.filename = file.filename;
+      alreadyKnown.size = file.size;
+      alreadyKnown.mime = resolveAttachmentMime(file.filename, file.mime);
+      alreadyKnown.isImage = isImageAttachment({
+        filename: file.filename,
+        isImage: !!file.isImage,
+        mime: alreadyKnown.mime
+      });
+
+      alreadyKnown.uploaderPeerId = file.uploaderPeerId ?? alreadyKnown.uploaderPeerId;
+      this.runtimeStore.touch();
+      void this.persistence.persistAttachmentMeta(alreadyKnown);
      return false;
    }

-    const attachment: Attachment = {
+    const normalizedMeta = normalizeAttachmentMeta({
      id: file.id,
      messageId,
      filename: file.filename,
      size: file.size,
      mime: file.mime,
-      isImage: isImageAttachment({
-        filename: file.filename,
-        isImage: !!file.isImage,
-        mime: file.mime
-      }),
-      uploaderPeerId: file.uploaderPeerId,
+      isImage: !!file.isImage,
+      uploaderPeerId: file.uploaderPeerId
+    });
+    const runtimeAttachment: Attachment = {
+      ...normalizedMeta,
      available: false,
      receivedBytes: 0
    };

-    list.push(attachment);
+    list.push(runtimeAttachment);
    this.runtimeStore.setAttachmentsForMessage(messageId, list);
    this.runtimeStore.touch();
-    void this.persistence.persistAttachmentMeta(attachment);
+    void this.persistence.persistAttachmentMeta(runtimeAttachment);

    return true;
  }
@@ -793,6 +823,10 @@ export class AttachmentTransferService {
    this.runtimeStore.touch();
    void this.persistence.persistAttachmentMeta(attachment);
    void this.announceLocalHost(attachment);
+
+    if (this.isPlayableMedia(attachment)) {
+      await this.hydratePlayableMediaAfterDiskReceive(attachment);
+    }
  }

  /**
@@ -0,0 +1,20 @@
+import { shouldResetStalledAttachmentDownload } from './attachment-autodownload.rules';
+
+describe('attachment autodownload rules', () => {
+  it('resets stalled partial downloads that are no longer actively transferring', () => {
+    expect(shouldResetStalledAttachmentDownload({
+      available: false,
+      receivedBytes: 128
+    }, false)).toBe(true);
+
+    expect(shouldResetStalledAttachmentDownload({
+      available: false,
+      receivedBytes: 128
+    }, true)).toBe(false);
+
+    expect(shouldResetStalledAttachmentDownload({
+      available: true,
+      receivedBytes: 128
+    }, false)).toBe(false);
+  });
+});
@@ -0,0 +1,8 @@
+export function shouldResetStalledAttachmentDownload(
+  attachment: Pick<{ available?: boolean; receivedBytes?: number }, 'available' | 'receivedBytes'>,
+  hasPendingRequest: boolean
+): boolean {
+  return !attachment.available &&
+    (attachment.receivedBytes ?? 0) > 0 &&
+    !hasPendingRequest;
+}
@@ -0,0 +1,41 @@
+import { resolveAttachmentMime } from './attachment-mime.rules';
+import { normalizeAttachmentMeta } from './attachment-normalize.rules';
+
+describe('attachment mime rules', () => {
+  it('keeps explicit audio and video mime types', () => {
+    expect(resolveAttachmentMime('song.mp3', 'audio/mpeg')).toBe('audio/mpeg');
+    expect(resolveAttachmentMime('clip.mp4', 'video/mp4')).toBe('video/mp4');
+  });
+
+  it('infers mime types from filenames when the declared type is generic', () => {
+    expect(resolveAttachmentMime('song.mp3', 'application/octet-stream')).toBe('audio/mpeg');
+    expect(resolveAttachmentMime('clip.webm', '')).toBe('video/webm');
+    expect(resolveAttachmentMime('photo.heic', 'application/octet-stream')).toBe('image/heic');
+  });
+
+  it('normalizes synced metadata so images and playable media classify correctly', () => {
+    expect(normalizeAttachmentMeta({
+      id: 'a1',
+      messageId: 'm1',
+      filename: 'clip.mp4',
+      size: 1024,
+      mime: 'application/octet-stream',
+      isImage: false
+    })).toEqual(expect.objectContaining({
+      mime: 'video/mp4',
+      isImage: false
+    }));
+
+    expect(normalizeAttachmentMeta({
+      id: 'a2',
+      messageId: 'm1',
+      filename: 'grid.png',
+      size: 512,
+      mime: 'application/octet-stream',
+      isImage: false
+    })).toEqual(expect.objectContaining({
+      mime: 'image/png',
+      isImage: true
+    }));
+  });
+});
@@ -0,0 +1,57 @@
+import { DEFAULT_ATTACHMENT_MIME_TYPE } from '../constants/attachment-transfer.constants';
+
+const GENERIC_MIME_TYPES = new Set([
+  '',
+  DEFAULT_ATTACHMENT_MIME_TYPE,
+  'binary/octet-stream'
+]);
+const EXTENSION_MIME_MAP: Record<string, string> = {
+  '.aac': 'audio/aac',
+  '.avi': 'video/x-msvideo',
+  '.bmp': 'image/bmp',
+  '.flac': 'audio/flac',
+  '.gif': 'image/gif',
+  '.heic': 'image/heic',
+  '.heif': 'image/heif',
+  '.jpeg': 'image/jpeg',
+  '.jpg': 'image/jpeg',
+  '.m4a': 'audio/mp4',
+  '.mkv': 'video/x-matroska',
+  '.mov': 'video/quicktime',
+  '.mp3': 'audio/mpeg',
+  '.mp4': 'video/mp4',
+  '.ogg': 'audio/ogg',
+  '.ogv': 'video/ogg',
+  '.png': 'image/png',
+  '.svg': 'image/svg+xml',
+  '.wav': 'audio/wav',
+  '.webm': 'video/webm',
+  '.webp': 'image/webp'
+};
+
+export function resolveAttachmentMime(filename: string, declaredType?: string | null): string {
+  const normalizedType = declaredType?.trim() ?? '';
+
+  if (normalizedType && !GENERIC_MIME_TYPES.has(normalizedType.toLowerCase())) {
+    return normalizedType;
+  }
+
+  const extension = extractFilenameExtension(filename);
+
+  if (extension) {
+    return EXTENSION_MIME_MAP[extension] ?? (normalizedType || DEFAULT_ATTACHMENT_MIME_TYPE);
+  }
+
+  return normalizedType || DEFAULT_ATTACHMENT_MIME_TYPE;
+}
+
+function extractFilenameExtension(filename: string): string | null {
+  const normalized = filename.trim().toLowerCase();
+  const extensionIndex = normalized.lastIndexOf('.');
+
+  if (extensionIndex <= 0) {
+    return null;
+  }
+
+  return normalized.slice(extensionIndex);
+}
@@ -0,0 +1,17 @@
+import { isImageAttachment } from './attachment-image.rules';
+import { resolveAttachmentMime } from './attachment-mime.rules';
+import type { AttachmentMeta } from '../models/attachment.model';
+
+export function normalizeAttachmentMeta<T extends AttachmentMeta>(meta: T): T {
+  const mime = resolveAttachmentMime(meta.filename, meta.mime);
+
+  return {
+    ...meta,
+    mime,
+    isImage: isImageAttachment({
+      filename: meta.filename,
+      isImage: meta.isImage,
+      mime
+    })
+  };
+}
@@ -3,6 +3,7 @@ import {
  isAttachmentPendingMediaHydration,
  isDirectMessageAttachmentRoomId,
  isPlayableAttachmentMedia,
+  needsAttachmentDisplayHydration,
  shouldCopyUploaderMediaToAppData,
  shouldCopyLargeUploaderFileToAppData,
  shouldStreamAttachmentReceiveToDisk,
@@ -91,6 +92,33 @@ describe('attachment logic', () => {
    }, capabilities)).toBe(true);
  });

+  it('combines inline image and playable media hydration needs', () => {
+    expect(needsAttachmentDisplayHydration({
+      filename: 'photo.png',
+      mime: 'image/png',
+      isImage: true,
+      available: false,
+      savedPath: '/data/photo.png'
+    })).toBe(true);
+
+    expect(needsAttachmentDisplayHydration({
+      filename: 'song.mp3',
+      mime: 'audio/mpeg',
+      isImage: false,
+      available: true,
+      savedPath: '/data/song.mp3'
+    })).toBe(true);
+
+    expect(needsAttachmentDisplayHydration({
+      filename: 'song.mp3',
+      mime: 'audio/mpeg',
+      isImage: false,
+      available: true,
+      objectUrl: 'file:///data/song.mp3',
+      savedPath: '/data/song.mp3'
+    })).toBe(false);
+  });
+
  it('identifies playable media pending hydration from disk paths', () => {
    expect(isPlayableAttachmentMedia({ mime: 'video/mp4' })).toBe(true);
    expect(isPlayableAttachmentMedia({ mime: 'image/png' })).toBe(false);
@@ -1,3 +1,4 @@
+import { isAttachmentPendingInlineHydration } from './attachment-image.rules';
 import { MAX_AUTO_SAVE_SIZE_BYTES } from '../constants/attachment.constants';
 import type { Attachment } from '../models/attachment.model';

@@ -32,6 +33,15 @@ export function isAttachmentPendingMediaHydration(
  return !!(attachment.savedPath?.trim() || attachment.filePath?.trim());
 }

+export function needsAttachmentDisplayHydration(
+  attachment: Pick<
+    Attachment,
+    'available' | 'filePath' | 'filename' | 'isImage' | 'mime' | 'objectUrl' | 'receivedBytes' | 'savedPath'
+  >
+): boolean {
+  return isAttachmentPendingInlineHydration(attachment) || isAttachmentPendingMediaHydration(attachment);
+}
+
 export function shouldAutoRequestWhenWatched(attachment: Attachment): boolean {
  return attachment.isImage ||
    (isAttachmentMedia(attachment) && attachment.size <= MAX_AUTO_SAVE_SIZE_BYTES);
@@ -51,8 +51,15 @@
                </button>
              }
              @case ('hydrating') {
-                <div class="flex aspect-square items-center justify-center rounded-md border border-border bg-secondary/40">
+                <div class="flex aspect-square flex-col items-center justify-center gap-2 rounded-md border border-border bg-secondary/40 p-3 text-center">
                  <div class="h-6 w-6 animate-spin rounded-full border-b-2 border-primary"></div>
+                  <button
+                    type="button"
+                    class="rounded-md bg-secondary px-2 py-1 text-xs text-foreground"
+                    (click)="retryGalleryImage(tile.attachment)"
+                  >
+                    {{ 'chat.message.retry' | translate }}
+                  </button>
                </div>
              }
              @case ('downloading') {
@@ -19,6 +19,7 @@ import {
  lucideX
 } from '@ng-icons/lucide';
 import { Attachment, AttachmentFacade } from '../../../../../attachment';
+import { isAttachmentPendingInlineHydration } from '../../../../../attachment/domain/logic/attachment-image.rules';
 import { canStepLightbox } from '../../../../domain/rules/chat-message-lightbox.rules';
 import { buildChatMessageGalleryTiles, type ChatMessageGalleryTile } from '../../../../domain/rules/chat-message-image-gallery.rules';
 import { AppI18nService, APP_TRANSLATE_IMPORTS } from '../../../../../../core/i18n';
@@ -127,6 +128,24 @@ export class ChatMessageOverlaysComponent implements OnDestroy {
    return `${state.index + 1} / ${state.attachments.length}`;
  });

+  private readonly syncGalleryHydration = effect(() => {
+    const attachments = this.galleryAttachments();
+
+    void this.attachmentsSvc.updated;
+
+    if (!attachments?.length) {
+      return;
+    }
+
+    for (const attachment of attachments) {
+      if (!isAttachmentPendingInlineHydration(attachment)) {
+        continue;
+      }
+
+      void this.attachmentsSvc.tryRestoreAttachmentFromLocal(attachment);
+    }
+  });
+
  private readonly appI18n = inject(AppI18nService);
  private readonly attachmentsSvc = inject(AttachmentFacade);
  private readonly LIGHTBOX_CONTROLS_IDLE_MS = 2200;
@@ -520,11 +520,23 @@ export class DmChatComponent {
      return;
    }

+    const displayableImages = attachments.filter((attachment) => attachment.available && attachment.objectUrl);
+
+    if (displayableImages.length > 0) {
+      this.attachments.pinDisplayBlobs(displayableImages);
+    }
+
    this.galleryMessageId.set(messageId);
    this.galleryAttachmentOrder.set(attachments.map((attachment) => attachment.id));
  }

  closeImageGallery(): void {
+    const gallery = this.galleryAttachments();
+
+    if (gallery) {
+      this.attachments.unpinDisplayBlobs(gallery);
+    }
+
    this.galleryMessageId.set(null);
    this.galleryAttachmentOrder.set([]);
  }