From ee71a382deb57cf723d0476d814ea44abd027a30 Mon Sep 17 00:00:00 2001 From: neo773 <62795688+neo773@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:15:13 +0530 Subject: [PATCH] IMAP support non RFC compliant servers (#22153) Some non complaint IMAP server don't send `UIDNEXT` UIDNEXT is the next message id you subtract with 1 to get total current messages This does a fallback to searching all UIDs and taking the highest Review in cubic --- .../imap-get-message-list.service.spec.ts | 45 +++++++ .../services/imap-get-message-list.service.ts | 18 ++- .../extract-mailbox-state.util.spec.ts | 126 ++++++++++++++++++ .../imap/utils/extract-mailbox-state.util.ts | 35 ++++- 4 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/__tests__/extract-mailbox-state.util.spec.ts diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.spec.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.spec.ts index bea5984745..2924d3e6f2 100644 --- a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.spec.ts +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.spec.ts @@ -30,6 +30,7 @@ const createMockFolder = ( describe('ImapGetMessageListService', () => { let service: ImapGetMessageListService; let imapClientProvider: ImapClientProvider; + let imapSyncService: ImapSyncService; const mockConnectedAccount: Pick< ConnectedAccountEntity, @@ -93,6 +94,7 @@ describe('ImapGetMessageListService', () => { service = module.get(ImapGetMessageListService); imapClientProvider = module.get(ImapClientProvider); + imapSyncService = module.get(ImapSyncService); }); afterEach(() => { @@ -229,4 +231,47 @@ describe('ImapGetMessageListService', () => { expect(imapClientProvider.closeClient).toHaveBeenCalledTimes(1); }); }); + + describe('incremental sync skip', () => { + const syncedFolder = createMockFolder({ + name: 'INBOX', + externalId: 'INBOX:12345', + isSynced: true, + syncCursor: JSON.stringify({ + highestUid: 99, + uidValidity: 12345, + modSeq: '1000', + }), + }); + + const runSync = () => + service.getMessageLists({ + connectedAccount: mockConnectedAccount, + messageChannel: { + syncCursor: '', + id: 'channel-1', + messageFolderImportPolicy: MessageFolderImportPolicy.ALL_FOLDERS, + }, + messageFolders: [syncedFolder], + }); + + it('skips folders whose cursor already covers the latest UID', async () => { + const [result] = await runSync(); + + expect(imapSyncService.syncFolder).not.toHaveBeenCalled(); + expect(result.messageExternalIds).toEqual([]); + }); + + it('does not skip when the server omits UIDNEXT on STATUS', async () => { + mockImapClient.status.mockResolvedValueOnce({ + uidValidity: 12345, + highestModseq: '1000', + }); + + const [result] = await runSync(); + + expect(imapSyncService.syncFolder).toHaveBeenCalledTimes(1); + expect(result.messageExternalIds).not.toEqual([]); + }); + }); }); diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.ts index 1cb6748bec..75cd138cfb 100644 --- a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.ts +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/services/imap-get-message-list.service.ts @@ -14,7 +14,7 @@ import { ImapClientProvider } from 'src/modules/messaging/message-import-manager import { ImapMessageListFetchErrorHandler } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-message-list-fetch-error-handler.service'; import { ImapSyncService } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-sync.service'; import { createSyncCursor } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/create-sync-cursor.util'; -import { extractMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util'; +import { resolveMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util'; import { getImapFolderPath } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/get-imap-folder-path.util'; import { parseSyncCursor } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-sync-cursor.util'; import { type GetMessageListsArgs } from 'src/modules/messaging/message-import-manager/types/get-message-lists-args.type'; @@ -116,7 +116,11 @@ export class ImapGetMessageListService { ); } - const mailboxState = extractMailboxState(mailbox); + const mailboxState = await resolveMailboxState( + client, + folderPath, + mailbox, + ); const { messageUids } = await this.imapSyncService.syncFolder( client, @@ -181,7 +185,15 @@ export class ImapGetMessageListService { return false; } - const uidNext = Number(status.uidNext ?? 1); + if (!isDefined(status.uidNext)) { + this.logger.debug( + `Folder ${folderPath}: Server missing UIDNEXT. Sync required.`, + ); + + return false; + } + + const uidNext = Number(status.uidNext); const uidValidity = Number(status.uidValidity); if (previousCursor.uidValidity !== uidValidity) { diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/__tests__/extract-mailbox-state.util.spec.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/__tests__/extract-mailbox-state.util.spec.ts new file mode 100644 index 0000000000..8af30ec6aa --- /dev/null +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/__tests__/extract-mailbox-state.util.spec.ts @@ -0,0 +1,126 @@ +import { type ImapFlow } from 'imapflow'; + +import { resolveMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util'; + +type MockClient = { + status: jest.Mock; + search: jest.Mock; +}; + +const createClient = (): MockClient => ({ + status: jest.fn(), + search: jest.fn(), +}); + +const asImapFlow = (client: MockClient) => client as unknown as ImapFlow; + +const createMailbox = ( + overrides: Partial> = {}, +) => + ({ + uidValidity: BigInt(100), + uidNext: 51, + highestModseq: BigInt(9), + ...overrides, + }) as NonNullable; + +describe('resolveMailboxState', () => { + it('uses the SELECT uidNext without extra round-trips when the server provides it', async () => { + const client = createClient(); + + const state = await resolveMailboxState( + asImapFlow(client), + 'INBOX', + createMailbox({ uidNext: 51 }), + ); + + expect(state).toEqual({ + uidValidity: 100, + uidNext: 51, + maxUid: 50, + highestModSeq: BigInt(9), + }); + expect(client.status).not.toHaveBeenCalled(); + expect(client.search).not.toHaveBeenCalled(); + }); + + it('falls back to STATUS when the server omits uidNext on SELECT', async () => { + const client = createClient(); + + client.status.mockResolvedValue({ uidNext: 71 }); + + const state = await resolveMailboxState( + asImapFlow(client), + 'INBOX', + createMailbox({ uidNext: undefined }), + ); + + expect(client.status).toHaveBeenCalledWith('INBOX', { uidNext: true }); + expect(client.search).not.toHaveBeenCalled(); + expect(state.uidNext).toBe(71); + expect(state.maxUid).toBe(70); + }); + + it('falls back to the highest live UID when both SELECT and STATUS omit uidNext', async () => { + const client = createClient(); + + client.status.mockResolvedValue({ uidNext: 0 }); + client.search.mockResolvedValue([3, 41, 17]); + + const state = await resolveMailboxState( + asImapFlow(client), + 'INBOX', + createMailbox({ uidNext: undefined }), + ); + + expect(client.search).toHaveBeenCalledWith({ uid: '1:*' }, { uid: true }); + expect(state.uidNext).toBe(42); + expect(state.maxUid).toBe(41); + }); + + it('handles a mailbox with 100k+ UIDs without overflowing the call stack', async () => { + const client = createClient(); + + client.status.mockResolvedValue({ uidNext: 0 }); + client.search.mockResolvedValue( + Array.from({ length: 200_000 }, (_, index) => index + 1), + ); + + const state = await resolveMailboxState( + asImapFlow(client), + 'INBOX', + createMailbox({ uidNext: undefined }), + ); + + expect(state.uidNext).toBe(200_001); + expect(state.maxUid).toBe(200_000); + }); + + it('treats an empty mailbox as uidNext 1 when no UID source is available', async () => { + const client = createClient(); + + client.status.mockResolvedValue({}); + client.search.mockResolvedValue([]); + + const state = await resolveMailboxState( + asImapFlow(client), + 'INBOX', + createMailbox({ uidNext: undefined }), + ); + + expect(state.uidNext).toBe(1); + expect(state.maxUid).toBe(0); + }); + + it('throws when the mailbox is not selected', async () => { + const client = createClient(); + + await expect( + resolveMailboxState( + asImapFlow(client), + 'INBOX', + true as unknown as NonNullable, + ), + ).rejects.toThrow('Invalid mailbox state'); + }); +}); diff --git a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util.ts b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util.ts index f009478a6a..f4f3abe47c 100644 --- a/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util.ts +++ b/packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util.ts @@ -1,4 +1,5 @@ import { type ImapFlow } from 'imapflow'; +import { isDefined } from 'twenty-shared/utils'; export type MailboxState = { uidValidity: number; @@ -7,14 +8,42 @@ export type MailboxState = { highestModSeq?: bigint; }; -export const extractMailboxState = ( +// David.fx and other non-RFC servers omit the required UIDNEXT on SELECT; +// fall back to STATUS, then the highest live UID, so the sync range isn't empty. +const resolveUidNext = async ( + client: ImapFlow, + folderPath: string, + mailboxUidNext: number | undefined, +): Promise => { + if (isDefined(mailboxUidNext)) { + return Number(mailboxUidNext); + } + + const status = await client.status(folderPath, { uidNext: true }); + const statusUidNext = Number(status.uidNext ?? 0); + + if (statusUidNext > 0) { + return statusUidNext; + } + + const uids = await client.search({ uid: '1:*' }, { uid: true }); + const highestUid = Array.isArray(uids) + ? uids.reduce((max, uid) => (uid > max ? uid : max), 0) + : 0; + + return highestUid + 1; +}; + +export const resolveMailboxState = async ( + client: ImapFlow, + folderPath: string, mailbox: NonNullable, -): MailboxState => { +): Promise => { if (typeof mailbox === 'boolean') { throw new Error('Invalid mailbox state'); } - const uidNext = Number(mailbox.uidNext ?? 1); + const uidNext = await resolveUidNext(client, folderPath, mailbox.uidNext); return { uidValidity: Number(mailbox.uidValidity ?? 0),