attachments fix

src/Command/ReclassifyAttachmentsCommand.php

@@ -0,0 +1,141 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Command;
+
+use App\Entity\Message;
+use App\Entity\MessagePart;
+use App\Service\Mail\InlineAttachmentDetector;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Style\SymfonyStyle;
+
+/**
+ * Repairs message parts stored while "has a Content-ID" was treated as
+ * "is inline". Those parts were saved as inline, so real attachments never
+ * appeared under the message and message.has_attachments stayed false.
+ *
+ * Re-runs InlineAttachmentDetector against the stored body HTML and rewrites
+ * is_inline / disposition, then recomputes message.has_attachments and
+ * thread.attachment_count. Idempotent.
+ */
+#[AsCommand(
+    name: 'app:attachments:reclassify',
+    description: 'Recompute inline/attachment classification for stored message parts',
+)]
+final class ReclassifyAttachmentsCommand extends Command
+{
+    private const int BATCH_SIZE = 200;
+
+    public function __construct(
+        private readonly EntityManagerInterface   $em,
+        private readonly InlineAttachmentDetector $inlineDetector,
+    ) {
+        parent::__construct();
+    }
+
+    protected function configure(): void
+    {
+        $this->addOption('dry-run', null, InputOption::VALUE_NONE, 'Report changes without writing them');
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output): int
+    {
+        $io     = new SymfonyStyle($input, $output);
+        $dryRun = true === $input->getOption('dry-run');
+
+        $messageIds = array_column(
+            $this->em->createQuery(
+                'SELECT DISTINCT IDENTITY(p.message) AS id FROM ' . MessagePart::class . ' p'
+            )->getArrayResult(),
+            'id',
+        );
+
+        $io->text(sprintf('%d messages with parts to inspect.', count($messageIds)));
+
+        $changedParts    = 0;
+        $changedMessages = 0;
+        $processed       = 0;
+
+        foreach (array_chunk($messageIds, self::BATCH_SIZE) as $chunk) {
+            /** @var list<Message> $messages */
+            $messages = $this->em->getRepository(Message::class)->findBy(['id' => $chunk]);
+
+            foreach ($messages as $message) {
+                $bodyHtml       = $message->getBodyHtml();
+                $hasAttachments = false;
+                $touched        = false;
+
+                foreach ($message->getMessageParts() as $part) {
+                    $isInline = $this->inlineDetector->isInline(
+                        $part->getDisposition(),
+                        $part->getContentId(),
+                        $bodyHtml,
+                    );
+
+                    if ($isInline !== $part->isInline()) {
+                        $part->setIsInline($isInline);
+                        $part->setDisposition($isInline ? 'inline' : 'attachment');
+                        ++$changedParts;
+                        $touched = true;
+                    }
+
+                    if (false === $isInline) {
+                        $hasAttachments = true;
+                    }
+                }
+
+                if ($hasAttachments !== $message->hasAttachments()) {
+                    $message->setHasAttachments($hasAttachments);
+                    $touched = true;
+                }
+
+                if (true === $touched) {
+                    ++$changedMessages;
+                }
+            }
+
+            $processed += count($messages);
+
+            if (false === $dryRun) {
+                $this->em->flush();
+            }
+
+            $this->em->clear();
+            $io->text(sprintf('  … %d/%d', $processed, count($messageIds)));
+        }
+
+        if (false === $dryRun) {
+            $this->recountThreads();
+        }
+
+        $io->success(sprintf(
+            '%s%d parts reclassified across %d messages.',
+            $dryRun ? '[dry-run] ' : '',
+            $changedParts,
+            $changedMessages,
+        ));
+
+        return Command::SUCCESS;
+    }
+
+    /**
+     * Thread counters are derived, so rebuild them from the repaired messages
+     * instead of trying to patch the deltas.
+     */
+    private function recountThreads(): void
+    {
+        $this->em->getConnection()->executeStatement(<<<'SQL'
+            UPDATE message_thread t
+            SET attachment_count = COALESCE((
+                SELECT COUNT(*) FROM message m
+                WHERE m.thread_id = t.id AND m.has_attachments = true
+            ), 0)
+        SQL);
+    }
+}

src/Service/Gmail/GmailMessageBuilder.php

@@ -9,6 +9,7 @@ use App\Entity\Label;
 use App\Entity\Message;
 use App\Entity\MessagePart;
 use App\Service\Label\LabelResolver;
+use App\Service\Mail\InlineAttachmentDetector;
 use DateTimeImmutable;
 use Doctrine\ORM\EntityManagerInterface;
 
@@ -38,6 +39,7 @@ final class GmailMessageBuilder
         private readonly EntityManagerInterface $em,
         private readonly GmailLabelResolver     $labelResolver,
         private readonly LabelResolver          $localLabelResolver,
+        private readonly InlineAttachmentDetector $inlineDetector,
     )
     {
     }
@@ -137,14 +139,18 @@ final class GmailMessageBuilder
 
         // ── Body + attachments ────────────────────────────────────────────────
 
-        [$bodyText, $bodyHtml, $hasAttachments] = $this->extractBody(
+        // Attachment parts are collected first and persisted afterwards: the
+        // inline/attachment decision needs the HTML body, which the same walk
+        // is still assembling.
+        [$bodyText, $bodyHtml, $attachmentParts] = $this->extractBody(
             $payload['payload'] ?? [],
-            $message,
         );
 
         $message->setBodyText($bodyText);
         $message->setBodyHtml($bodyHtml);
-        $message->setHasAttachments($hasAttachments);
+        $message->setHasAttachments(
+            $this->persistAttachmentStubs($attachmentParts, $message, $bodyHtml)
+        );
         $message->setSyncedAt(new DateTimeImmutable());
 
         return $message;
@@ -210,16 +216,13 @@ final class GmailMessageBuilder
      * Walk the MIME tree and extract text/html body parts and attachments.
      *
      * @param array<string,mixed> $part
-     * @return array{string, string, bool}  [bodyText, bodyHtml, hasAttachments]
+     * @return array{string, string, list<array<string,mixed>>}  [bodyText, bodyHtml, attachmentParts]
      */
-    private function extractBody(
-        array   $part,
-        Message $message,
-    ): array
+    private function extractBody(array $part): array
     {
         $bodyText = '';
         $bodyHtml = '';
-        $hasAttachments = false;
+        $attachmentParts = [];
 
         $mimeType = strtolower((string)($part['mimeType'] ?? ''));
 
@@ -239,65 +242,68 @@ final class GmailMessageBuilder
             $hasContentId = '' !== trim(($partHeaders['content-id'] ?? ''), '<> ');
 
             if ('' !== $filename || true === $hasContentId) {
-                $isInline = $this->persistAttachmentStub($part, $message);
-
-                if (false === $isInline) {
-                    $hasAttachments = true;
-                }
+                $attachmentParts[] = $part;
             }
         }
 
         foreach ($part['parts'] ?? [] as $subPart) {
-            [$t, $h, $a] = $this->extractBody($subPart, $message);
+            [$t, $h, $a] = $this->extractBody($subPart);
             if ('' === $bodyText) {
                 $bodyText = $t;
             }
             if ('' === $bodyHtml) {
                 $bodyHtml = $h;
             }
-            if (true === $a) {
+
+            $attachmentParts = array_merge($attachmentParts, $a);
+        }
+
+        return [$bodyText, $bodyHtml, $attachmentParts];
+    }
+
+    /**
+     * Persist MessagePart stubs for the collected attachment parts. Bytes are
+     * fetched lazily by AttachmentResolver on first access.
+     *
+     * @param list<array<string,mixed>> $parts
+     * @return bool  true if at least one part is a real (non-inline) attachment
+     */
+    private function persistAttachmentStubs(array $parts, Message $message, string $bodyHtml): bool
+    {
+        $hasAttachments = false;
+
+        foreach ($parts as $part) {
+            $partHeaders = $this->indexHeaders($part['headers'] ?? []);
+            $filename = (string)($part['filename'] ?? 'attachment');
+            $contentType = (string)($part['mimeType'] ?? 'application/octet-stream');
+            $attachmentId = (string)($part['body']['attachmentId'] ?? '');
+            $size = (int)($part['body']['size'] ?? 0);
+
+            $contentId = $this->inlineDetector->normalizeContentId($partHeaders['content-id'] ?? null);
+            $isInline = $this->inlineDetector->isInline(
+                $partHeaders['content-disposition'] ?? null,
+                $contentId,
+                $bodyHtml,
+            );
+
+            $mp = new MessagePart()
+                ->setMessage($message)
+                ->setContentType($contentType)
+                ->setFilename($filename)
+                ->setContentId('' !== $contentId ? $contentId : null)
+                ->setDisposition($isInline ? 'inline' : 'attachment')
+                ->setSize($size)
+                ->setStoragePath('gmail://' . $attachmentId)
+                ->setIsInline($isInline);
+
+            $this->em->persist($mp);
+
+            if (false === $isInline) {
                 $hasAttachments = true;
             }
         }
 
-        return [$bodyText, $bodyHtml, $hasAttachments];
-    }
-
-    /**
-     * Persist a MessagePart stub for an attachment. Bytes are fetched lazily by
-     * AttachmentResolver on first access.
-     *
-     * @param array<string,mixed> $part
-     * @return bool  true if the part is inline (referenced from the body)
-     */
-    private function persistAttachmentStub(
-        array   $part,
-        Message $message,
-    ): bool
-    {
-        $partHeaders = $this->indexHeaders($part['headers'] ?? []);
-        $filename = (string)($part['filename'] ?? 'attachment');
-        $contentType = (string)($part['mimeType'] ?? 'application/octet-stream');
-        $attachmentId = (string)($part['body']['attachmentId'] ?? '');
-        $size = (int)($part['body']['size'] ?? 0);
-
-        $contentId = trim($partHeaders['content-id'] ?? '', '<> ');
-        $disposition = strtolower($partHeaders['content-disposition'] ?? '');
-        $isInline = true === str_contains($disposition, 'inline') || '' !== $contentId;
-
-        $mp = new MessagePart()
-            ->setMessage($message)
-            ->setContentType($contentType)
-            ->setFilename($filename)
-            ->setContentId('' !== $contentId ? $contentId : null)
-            ->setDisposition($isInline ? 'inline' : 'attachment')
-            ->setSize($size)
-            ->setStoragePath('gmail://' . $attachmentId)
-            ->setIsInline($isInline);
-
-        $this->em->persist($mp);
-
-        return $isInline;
+        return $hasAttachments;
     }
 
     private function decodeMimeHeader(string $value): string

src/Service/Graph/GraphMessageBuilder.php

@@ -7,6 +7,7 @@ namespace App\Service\Graph;
 use App\Entity\Account;
 use App\Entity\Message;
 use App\Entity\MessagePart;
+use App\Service\Mail\InlineAttachmentDetector;
 use DateTimeImmutable;
 use Doctrine\ORM\EntityManagerInterface;
 
@@ -46,6 +47,7 @@ final class GraphMessageBuilder
     public function __construct(
         private readonly EntityManagerInterface $em,
         private readonly GraphFolderResolver    $folderResolver,
+        private readonly InlineAttachmentDetector $inlineDetector,
     ) {}
 
     /**
@@ -210,8 +212,12 @@ final class GraphMessageBuilder
                 continue;
             }
 
-            $contentId = trim((string) ($attachment['contentId'] ?? ''), '<> ');
-            $isInline  = true === ($attachment['isInline'] ?? false) || '' !== $contentId;
+            $contentId = $this->inlineDetector->normalizeContentId($attachment['contentId'] ?? null);
+            $isInline  = $this->inlineDetector->isInline(
+                true === ($attachment['isInline'] ?? false) ? 'inline' : 'attachment',
+                $contentId,
+                $message->getBodyHtml(),
+            );
 
             $part = new MessagePart()
                 ->setMessage($message)

src/Service/Imap/MessageSyncer.php

@@ -12,6 +12,7 @@ use App\Jmap\State\StateManager;
 use App\Repository\ContactRepository;
 use App\Repository\MailboxRepository;
 use App\Repository\MessageRepository;
+use App\Service\Mail\InlineAttachmentDetector;
 use App\Service\Mail\MailBodySanitizer;
 use App\Service\Mail\MessageCategorizer;
 use App\Service\Mail\RawMessageResolver;
@@ -37,6 +38,7 @@ class MessageSyncer
         private readonly ContactRepository       $contactRepository,
         private readonly StateManager            $stateManager,
         private readonly RawMessageResolver      $rawResolver,
+        private readonly InlineAttachmentDetector $inlineDetector,
     ) {}
 
     public function syncMailbox(Mailbox $mailbox, Client $client): void
@@ -335,18 +337,26 @@ class MessageSyncer
 
         // Attachments
         $attachments = $imapMessage->getAttachments();
-        $message->setHasAttachments($attachments->isNotEmpty());
         $message->setSyncedAt(new DateTimeImmutable());
 
+        $hasAttachments = false;
+
         foreach ($attachments as $attachment) {
-            $this->persistAttachment($attachment, $message, $accountId);
+            if (false === $this->persistAttachment($attachment, $message, $accountId)) {
+                $hasAttachments = true;
+            }
         }
 
+        $message->setHasAttachments($hasAttachments);
+
 
         return $message;
     }
 
-    private function persistAttachment(mixed $attachment, Message $message, int $accountId): void
+    /**
+     * @return bool  true if the part is inline (embedded in the HTML body)
+     */
+    private function persistAttachment(mixed $attachment, Message $message, int $accountId): bool
     {
         $filename = $attachment->getFilename() ?? ('attachment_' . uniqid());
         $content  = $attachment->getContent();
@@ -363,19 +373,25 @@ class MessageSyncer
         $part->setMessage($message);
         $part->setContentType($attachment->getContentType() ?? 'application/octet-stream');
         $part->setFilename($filename);
-        $part->setDisposition($attachment->getDisposition() ?? 'attachment');
         $part->setSize(strlen($content));
         $part->setStoragePath($storagePath);
 
-        $contentId     = $attachment->getId();
-        $normalizedCid = null !== $contentId ? trim((string) $contentId, '<> ') : '';
-        $part->setContentId('' !== $normalizedCid ? $normalizedCid : null);
-
-        $part->setIsInline(
-            ($attachment->getDisposition() ?? '') === 'inline' || '' !== $normalizedCid
+        // getId() falls back to a content hash when the part has no Content-ID,
+        // so it only counts as a cid when the HTML body actually references it.
+        $normalizedCid = $this->inlineDetector->normalizeContentId((string) $attachment->getId());
+        $isInline      = $this->inlineDetector->isInline(
+            $attachment->getDisposition(),
+            $normalizedCid,
+            $message->getBodyHtml(),
         );
 
+        $part->setContentId('' !== $normalizedCid ? $normalizedCid : null);
+        $part->setDisposition($isInline ? 'inline' : 'attachment');
+        $part->setIsInline($isInline);
+
         $this->em->persist($part);
+
+        return $isInline;
     }
 
     private function formatAddresses(mixed $attribute): array

src/Service/Mail/InlineAttachmentDetector.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Mail;
+
+/**
+ * Decides whether a MIME part is an inline body asset or a real attachment.
+ *
+ * A Content-ID alone means nothing: Gmail's web composer stamps
+ * "Content-ID: <f_xxxx>" on every attachment it uploads, and Webklex's
+ * Attachment::getId() falls back to a content hash when the part carries no
+ * Content-ID at all. Treating "has a cid" as "is inline" therefore hides
+ * ordinary attachments from the UI.
+ *
+ * The body is the authority: a part is inline when the HTML actually
+ * references its cid. Content-Disposition only decides the cases the body
+ * cannot (no HTML part, or a cid the body never mentions).
+ */
+final class InlineAttachmentDetector
+{
+    public function isInline(
+        ?string $disposition,
+        ?string $contentId,
+        ?string $bodyHtml,
+    ): bool {
+        $contentId = $this->normalizeContentId($contentId);
+
+        if (true === $this->isReferencedBy($contentId, (string) $bodyHtml)) {
+            return true;
+        }
+
+        $disposition = strtolower(trim((string) $disposition));
+
+        if (true === str_contains($disposition, 'attachment')) {
+            return false;
+        }
+
+        return str_contains($disposition, 'inline');
+    }
+
+    public function normalizeContentId(?string $contentId): string
+    {
+        return trim((string) $contentId, "<> \t\r\n");
+    }
+
+    /**
+     * True when the HTML body embeds this part via a cid: URL.
+     */
+    private function isReferencedBy(string $contentId, string $bodyHtml): bool
+    {
+        if ('' === $contentId || '' === $bodyHtml) {
+            return false;
+        }
+
+        return 1 === preg_match(
+            '/cid:\s*' . preg_quote($contentId, '/') . '\b/i',
+            $bodyHtml,
+        );
+    }
+}

templates/_partials/_thread_row.html.twig

@@ -34,6 +34,8 @@ inside the reading pane.
     {% set draft = latest and latest.isDraft and item.messageCount == 1 %}
     {% set draftId = draft ? latest.id : null %}
 
+    {% set hasAttachment = item.attachmentCount > 0 %}
+
     {% set href = path('app_mail_thread', { id: item.id }) %}
     {% set rowId = item.id %}
 {% else %}
@@ -46,6 +48,8 @@ inside the reading pane.
     {% set sender = item.fromName ?: item.fromAddress %}
     {% set snippet = (item.bodyText ?? '')|striptags|slice(0, 100) %}
 
+    {% set hasAttachment = item.hasAttachments %}
+
     {% set draft = item.isDraft %}
     {% set draftId = item.id %}
 
@@ -109,7 +113,6 @@ inside the reading pane.
                 <span class="truncate text-sm {{ unread ? 'font-semibold text-gray-900 dark:text-gray-100' : 'font-normal text-gray-600 dark:text-gray-400' }}">
                     {{ sender }}
                 </span>
-
                 {# Date — mobile only (top-right), hidden on desktop #}
                 <span class="md:hidden flex-shrink-0 ml-2 text-xs text-gray-400 dark:text-gray-600">
                     {% if date %}
@@ -135,13 +138,19 @@ inside the reading pane.
                 </span>
                 <span class="text-gray-400 dark:text-gray-600"> — {{ snippet }}</span>
             </span>
-
         </div>
 
         {# Date + hover actions slot — desktop only #}
         <div class="hidden md:flex flex-shrink-0 w-36 relative h-full items-center">
 
             <span class="absolute inset-0 flex items-center justify-end group-hover:invisible text-xs text-gray-400 dark:text-gray-600">
+                                       {% if hasAttachment %}
+                                           <span class="flex-shrink-0 mr-2 text-gray-400 dark:text-gray-600"
+                                                 title="{{ 'thread_row.has_attachment'|trans }}">
+                    <i class="fa-solid fa-paperclip text-[12px]" aria-hidden="true"></i>
+                    <span class="sr-only">{{ 'thread_row.has_attachment'|trans }}</span>
+                </span>
+                                       {% endif %}
                 {% if date %}
                     {% if date|date('Y-m-d') == 'now'|date('Y-m-d') %}
                         {{ date|date('g:i a') }}

templates/mail/_thread_message.html.twig

@@ -132,6 +132,7 @@ Variables:
                 <div class="flex flex-wrap gap-2">
                     {% for part in message.messageParts %}
                         {% if not part.isInline %}
+
                             <a
                                 href="{{ path('app_mail_attachment', { id: part.id }) }}?download=1"
                                 class="inline-flex items-center gap-2 px-3 py-2 rounded-lg text-xs

translations/messages.de.yaml

@@ -509,6 +509,7 @@ thread_row:
         mark_read: Als gelesen markieren
         mark_unread: Als ungelesen markieren
         snooze: Zurückstellen
+    has_attachment: Enthält Anhang
     message_count: "({ count })"
     no_subject: (kein Betreff)
     select: Auswählen

translations/messages.en.yaml

@@ -497,6 +497,7 @@ thread_row:
         mark_read: Mark as read
         mark_unread: Mark as unread
         snooze: Snooze
+    has_attachment: Has attachment
     message_count: "({ count })"
     no_subject: (no subject)
     select: Select

translations/messages.en_PI.yaml

@@ -497,6 +497,7 @@ thread_row:
         mark_read: Mark as read
         mark_unread: Mark as unread
         snooze: Let it drift
+    has_attachment: Carries cargo
     message_count: "({ count })"
     no_subject: (no subject)
     select: Pick it