From c6364d40e67954f470960127437e489fd99631bc Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Fri, 17 Jul 2026 22:50:02 +0200 Subject: [PATCH] fix: mail sync duplicates, sent folder, delimiter detection, folder move tracking - Add imap_uid column + unique index for proper deduplication - Use deterministic message_id for emails without Message-ID header - Dedup by (account_id, folder_id, imap_uid) first, message_id fallback - Track folder moves: update folder_id when email appears in different folder - Fix Sent folder lookup: check Sent/INBOX.Sent/Sent Items/Sent Mail + ILIKE - Detect IMAP delimiter from LIST response instead of hardcoding dot - Use detected delimiter in stale folder name migration - Migration 0004_imap_uid.sql with imap_uid column and unique index --- .../mail/migrations/0004_imap_uid.sql | 5 ++ app/plugins/builtins/mail/models.py | 1 + app/plugins/builtins/mail/services.py | 86 +++++++++++++++---- 3 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 app/plugins/builtins/mail/migrations/0004_imap_uid.sql diff --git a/app/plugins/builtins/mail/migrations/0004_imap_uid.sql b/app/plugins/builtins/mail/migrations/0004_imap_uid.sql new file mode 100644 index 0000000..8588012 --- /dev/null +++ b/app/plugins/builtins/mail/migrations/0004_imap_uid.sql @@ -0,0 +1,5 @@ +-- Add imap_uid column to mails table for proper deduplication +ALTER TABLE mails ADD COLUMN IF NOT EXISTS imap_uid VARCHAR(20); +CREATE INDEX IF NOT EXISTS ix_mails_imap_uid ON mails(account_id, imap_uid); +-- Unique constraint to prevent duplicates per folder +CREATE UNIQUE INDEX IF NOT EXISTS uq_mails_account_folder_uid ON mails(account_id, folder_id, imap_uid) WHERE imap_uid IS NOT NULL; diff --git a/app/plugins/builtins/mail/models.py b/app/plugins/builtins/mail/models.py index 96745ad..f4f674a 100644 --- a/app/plugins/builtins/mail/models.py +++ b/app/plugins/builtins/mail/models.py @@ -111,6 +111,7 @@ class Mail(Base, TenantMixin): ForeignKey("mail_folders.id", ondelete="CASCADE"), nullable=False, ) + imap_uid: Mapped[str | None] = mapped_column(String(20), nullable=True) message_id: Mapped[str] = mapped_column(String(512), nullable=False) thread_id: Mapped[str] = mapped_column(String(512), nullable=False, default="") in_reply_to: Mapped[str | None] = mapped_column(String(512), nullable=True) diff --git a/app/plugins/builtins/mail/services.py b/app/plugins/builtins/mail/services.py index 02dc84f..80c2a57 100644 --- a/app/plugins/builtins/mail/services.py +++ b/app/plugins/builtins/mail/services.py @@ -411,14 +411,18 @@ def _get_german_folder_name(imap_name: str) -> str: return imap_name -def _parse_imap_list_response(response) -> list[tuple[str, str]]: +def _parse_imap_list_response(response) -> tuple[list[tuple[str, str]], str]: """Parse IMAP LIST response into list of (flags, folder_name) tuples. Handles both "/" and "." delimiters. The IMAP LIST response format is: * LIST (\\HasChildren) "." "INBOX" * LIST (\\HasNoChildren) "." "INBOX.Sent" + + Returns (folders, delimiter) where delimiter is the hierarchy separator + detected from the LIST response (defaults to '.' if not found). """ folders: list[tuple[str, str]] = [] + delimiter = '.' lines = response.lines if hasattr(response, 'lines') else response for line in lines: if isinstance(line, (bytes, bytearray)): @@ -441,7 +445,7 @@ def _parse_imap_list_response(response) -> list[tuple[str, str]]: folder_name = parts[-2] if len(parts) >= 2 else '' if folder_name: folders.append(('', folder_name)) - return folders + return folders, delimiter def _build_folder_hierarchy( @@ -449,15 +453,15 @@ def _build_folder_hierarchy( account_id: uuid.UUID, tenant_id: uuid.UUID, existing_folders: dict[str, MailFolder], + delimiter: str = '.', ) -> list[MailFolder]: """Create or update MailFolder records from IMAP LIST response. - Handles dot-delimited hierarchies (e.g. INBOX.Sent → parent=INBOX). + Handles delimiter-separated hierarchies (e.g. INBOX.Sent → parent=INBOX). Updates existing folders in-place (name, is_standard, parent_id) so that stale DB records with wrong imap_name values get corrected. """ result: list[MailFolder] = [] - delimiter = '.' # First pass: create or update folder records for flags, imap_name in imap_folders: @@ -598,7 +602,7 @@ async def imap_sync_account( try: # 1) LIST all folders from IMAP server list_response = await client.list('""', '"*"') - imap_folders = _parse_imap_list_response(list_response) + imap_folders, imap_delimiter = _parse_imap_list_response(list_response) # If LIST returned nothing, fall back to standard folders (dot-delimited) if not imap_folders: @@ -627,10 +631,10 @@ async def imap_sync_account( imap_names_from_server = {name for _, name in imap_folders if name} for db_folder in existing_db_folders: if db_folder.imap_name not in imap_names_from_server: - # Try matching by leaf component - leaf = db_folder.imap_name.rsplit(".", 1)[-1] + # Try matching by leaf component using detected delimiter + leaf = db_folder.imap_name.rsplit(imap_delimiter, 1)[-1] for srv_name in imap_names_from_server: - srv_leaf = srv_name.rsplit(".", 1)[-1] + srv_leaf = srv_name.rsplit(imap_delimiter, 1)[-1] if srv_leaf.lower() == leaf.lower(): db_folder.imap_name = srv_name existing_by_imap[srv_name] = db_folder @@ -638,7 +642,7 @@ async def imap_sync_account( # 3) Create/update folders in DB db_folders = _build_folder_hierarchy( - imap_folders, account.id, tenant_id, existing_by_imap + imap_folders, account.id, tenant_id, existing_by_imap, imap_delimiter ) for folder in db_folders: if folder.id is None: @@ -646,12 +650,11 @@ async def imap_sync_account( await db.flush() # 3a) Re-set parent_id now that new folders have IDs after flush - delimiter = '.' folder_by_imap_post_flush = {f.imap_name: f for f in db_folders} for folder in db_folders: - if delimiter in folder.imap_name: - parts = folder.imap_name.split(delimiter) - parent_imap = delimiter.join(parts[:-1]) + if imap_delimiter in folder.imap_name: + parts = folder.imap_name.split(imap_delimiter) + parent_imap = imap_delimiter.join(parts[:-1]) if parent_imap in folder_by_imap_post_flush: parent = folder_by_imap_post_flush[parent_imap] if parent.id: @@ -751,7 +754,12 @@ async def imap_sync_account( except Exception: return value - message_id = msg.get("Message-ID", make_msgid()) + raw_msg_id = msg.get("Message-ID") + if raw_msg_id: + message_id = raw_msg_id + else: + # Deterministic ID so re-syncs don't create duplicates + message_id = f"generated-{account.id}-{folder.id}-{uid_str}" subject = _decode_mime_header(msg.get("Subject", "")) from_addr = _decode_mime_header(msg.get("From", "")) to_addrs = _decode_mime_header(msg.get("To", "")) @@ -774,19 +782,44 @@ async def imap_sync_account( # Compute thread_id from References/In-Reply-To thread_id = _compute_thread_id(message_id, refs, in_reply_to) - # Check if email already exists (by message_id + folder) + # Dedup: first check by (account_id, folder_id, imap_uid), + # then fall back to (account_id, message_id) for mails synced + # before the imap_uid column existed. existing_mail = ( await db.execute( select(Mail).where( and_( Mail.account_id == account.id, - Mail.message_id == message_id, + Mail.folder_id == folder.id, + Mail.imap_uid == uid_str, Mail.tenant_id == tenant_id, ) ) ) ).scalar_one_or_none() + + if not existing_mail: + existing_mail = ( + await db.execute( + select(Mail).where( + and_( + Mail.account_id == account.id, + Mail.message_id == message_id, + Mail.tenant_id == tenant_id, + ) + ) + ) + ).scalar_one_or_none() + if existing_mail: + # Track folder moves: update folder_id and imap_uid if the + # mail now appears in a different IMAP folder. + if existing_mail.folder_id != folder.id: + existing_mail.folder_id = folder.id + existing_mail.imap_uid = uid_str + elif not existing_mail.imap_uid: + existing_mail.imap_uid = uid_str + # Save attachments for existing emails that have has_attachments but no records if attachments and existing_mail.has_attachments: existing_att_count = ( @@ -827,6 +860,7 @@ async def imap_sync_account( tenant_id=tenant_id, account_id=account.id, folder_id=folder.id, + imap_uid=uid_str, message_id=message_id, thread_id=thread_id, in_reply_to=in_reply_to, @@ -1045,18 +1079,34 @@ async def send_mail_via_smtp( await smtp.send_message(msg, recipients=recipients) await smtp.quit() - # Store sent mail in Sent folder + # Store sent mail in Sent folder — flexible lookup to handle + # different IMAP naming conventions (Sent, INBOX.Sent, Sent Items, etc.) sent_folder = ( await db.execute( select(MailFolder).where( and_( MailFolder.account_id == account.id, - MailFolder.imap_name == "Sent", + MailFolder.imap_name.in_( + ["Sent", "INBOX.Sent", "Sent Items", "Sent Mail"] + ), ) ) ) ).scalar_one_or_none() + if not sent_folder: + sent_folder = ( + await db.execute( + select(MailFolder).where( + and_( + MailFolder.account_id == account.id, + MailFolder.is_standard == True, + MailFolder.imap_name.ilike("%sent%"), + ) + ) + ) + ).scalar_one_or_none() + if sent_folder: thread_id = _compute_thread_id(msg_id, references_header or "", in_reply_to) sent_mail = Mail(