fix: IMAP skip no-select flag folders properly (#19402)

Tested with a dovecot server running in Docker with synthetic seed <img width="546" height="54" alt="image" src="https://github.com/user-attachments/assets/81cbeae6-9cb6-406b-846a-209af403385f" /> /closes #19090
2026-04-08 10:24:01 +05:30
parent c91d642f29
commit 20adb86917
2 changed files with 232 additions and 24 deletions
@@ -0,0 +1,194 @@
+import { Test, type TestingModule } from '@nestjs/testing';
+
+import { type ImapFlow, type ListResponse } from 'imapflow';
+import {
+  ConnectedAccountProvider,
+  MessageFolderImportPolicy,
+} from 'twenty-shared/types';
+
+import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { type MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
+import { ImapGetAllFoldersService } from 'src/modules/messaging/message-folder-manager/drivers/imap/services/imap-get-all-folders.service';
+import { ImapClientProvider } from 'src/modules/messaging/message-import-manager/drivers/imap/providers/imap-client.provider';
+import { ImapFindSentFolderService } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-find-sent-folder.service';
+
+const createMockMailbox = (
+  overrides: Partial<ListResponse> & Pick<ListResponse, 'path'>,
+): ListResponse => ({
+  pathAsListed: overrides.path,
+  name: overrides.path.split('.').pop() ?? overrides.path,
+  delimiter: '.',
+  parent: [],
+  parentPath: '',
+  flags: new Set<string>(),
+  listed: true,
+  subscribed: true,
+  ...overrides,
+});
+
+const CONNECTED_ACCOUNT: Pick<
+  ConnectedAccountEntity,
+  'id' | 'provider' | 'connectionParameters' | 'handle'
+> = {
+  id: 'account-1',
+  provider: ConnectedAccountProvider.IMAP_SMTP_CALDAV,
+  connectionParameters: {},
+  handle: 'test@example.com',
+};
+
+const MESSAGE_CHANNEL: Pick<MessageChannelEntity, 'messageFolderImportPolicy'> =
+  {
+    messageFolderImportPolicy: MessageFolderImportPolicy.ALL_FOLDERS,
+  };
+
+describe('ImapGetAllFoldersService', () => {
+  let service: ImapGetAllFoldersService;
+  let mockImapClient: jest.Mocked<Pick<ImapFlow, 'list' | 'status'>>;
+  let imapFindSentFolderService: jest.Mocked<ImapFindSentFolderService>;
+
+  beforeEach(async () => {
+    mockImapClient = {
+      list: jest.fn().mockResolvedValue([]),
+      status: jest.fn(),
+    };
+
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        ImapGetAllFoldersService,
+        {
+          provide: ImapClientProvider,
+          useValue: {
+            getClient: jest.fn().mockResolvedValue(mockImapClient),
+            closeClient: jest.fn().mockResolvedValue(undefined),
+          },
+        },
+        {
+          provide: ImapFindSentFolderService,
+          useValue: {
+            findSentFolder: jest.fn().mockResolvedValue(null),
+          },
+        },
+      ],
+    }).compile();
+
+    service = module.get(ImapGetAllFoldersService);
+    imapFindSentFolderService = module.get(ImapFindSentFolderService);
+  });
+
+  describe('Noselect folder handling', () => {
+    it('should not issue STATUS against a \\Noselect folder', async () => {
+      const mailboxList = [
+        createMockMailbox({ path: 'INBOX' }),
+        createMockMailbox({
+          path: 'INBOX.Others',
+          flags: new Set(['\\Noselect']),
+        }),
+        createMockMailbox({
+          path: 'INBOX.Others.Sub1',
+          name: 'Sub1',
+          parentPath: 'INBOX.Others',
+        }),
+      ];
+
+      mockImapClient.list.mockResolvedValue(mailboxList);
+      mockImapClient.status.mockImplementation(async (path: string) => {
+        const uidMap: Record<string, bigint> = {
+          INBOX: BigInt(1),
+          'INBOX.Others.Sub1': BigInt(2),
+        };
+
+        if (path in uidMap) {
+          return { uidValidity: uidMap[path] } as any;
+        }
+        throw new Error(`Mailbox doesn't exist: ${path}`);
+      });
+
+      const result = await service.getAllMessageFolders(
+        CONNECTED_ACCOUNT,
+        MESSAGE_CHANNEL,
+      );
+
+      expect(mockImapClient.status).not.toHaveBeenCalledWith(
+        'INBOX.Others',
+        expect.anything(),
+      );
+
+      const paths = result.map((f) => f.externalId?.split(':')[0]);
+
+      expect(paths).toContain('INBOX');
+      expect(paths).toContain('INBOX.Others.Sub1');
+      expect(paths).not.toContain('INBOX.Others');
+    });
+
+    it('should preserve parent references for children of \\Noselect folders', async () => {
+      const mailboxList = [
+        createMockMailbox({
+          path: 'INBOX.Others',
+          flags: new Set(['\\Noselect']),
+        }),
+        createMockMailbox({
+          path: 'INBOX.Others.Sub1',
+          name: 'Sub1',
+          parentPath: 'INBOX.Others',
+        }),
+        createMockMailbox({
+          path: 'INBOX.Others.Sub2',
+          name: 'Sub2',
+          parentPath: 'INBOX.Others',
+        }),
+      ];
+
+      mockImapClient.list.mockResolvedValue(mailboxList);
+      mockImapClient.status.mockImplementation(async (path: string) => {
+        const uidMap: Record<string, bigint> = {
+          'INBOX.Others.Sub1': BigInt(10),
+          'INBOX.Others.Sub2': BigInt(11),
+        };
+
+        return { uidValidity: uidMap[path] } as any;
+      });
+
+      const result = await service.getAllMessageFolders(
+        CONNECTED_ACCOUNT,
+        MESSAGE_CHANNEL,
+      );
+
+      expect(result).toHaveLength(2);
+
+      for (const folder of result) {
+        expect(folder.parentFolderId).toBe('INBOX.Others');
+      }
+    });
+
+    it('should exclude \\Noselect sent folder from results and skip STATUS', async () => {
+      const mailboxList = [
+        createMockMailbox({ path: 'INBOX' }),
+        createMockMailbox({
+          path: 'Sent',
+          flags: new Set(['\\Noselect']),
+        }),
+      ];
+
+      mockImapClient.list.mockResolvedValue(mailboxList);
+      mockImapClient.status.mockImplementation(async () => {
+        return { uidValidity: BigInt(1) } as any;
+      });
+
+      imapFindSentFolderService.findSentFolder.mockResolvedValue({
+        path: 'Sent',
+        name: 'Sent',
+      });
+
+      const result = await service.getAllMessageFolders(
+        CONNECTED_ACCOUNT,
+        MESSAGE_CHANNEL,
+      );
+
+      expect(mockImapClient.status).not.toHaveBeenCalledWith(
+        'Sent',
+        expect.anything(),
+      );
+      expect(result.find((f) => f.isSentFolder)).toBeUndefined();
+    });
+  });
+});
@@ -8,8 +8,8 @@ import {
  MessageFolderDriver,
 } from 'src/modules/messaging/message-folder-manager/interfaces/message-folder-driver.interface';

-import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
 import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
 import { shouldCreateFolderByDefault } from 'src/modules/messaging/message-folder-manager/utils/should-create-folder-by-default.util';
 import { shouldSyncFolderByDefault } from 'src/modules/messaging/message-folder-manager/utils/should-sync-folder-by-default.util';
 import { ImapClientProvider } from 'src/modules/messaging/message-import-manager/drivers/imap/providers/imap-client.provider';
@@ -66,11 +66,16 @@ export class ImapGetAllFoldersService implements MessageFolderDriver {
    const sentFolder =
      await this.imapFindSentFolderService.findSentFolder(client);

-    if (isDefined(sentFolder)) {
-      const sentMailbox = mailboxList.find((m) => m.path === sentFolder.path);
-      const uidValidity = sentMailbox
-        ? await this.getUidValidity(client, sentMailbox)
-        : null;
+    const sentMailbox = isDefined(sentFolder)
+      ? mailboxList.find((mailbox) => mailbox.path === sentFolder.path)
+      : undefined;
+
+    if (
+      isDefined(sentFolder) &&
+      isDefined(sentMailbox) &&
+      this.isMailboxSelectable(sentMailbox)
+    ) {
+      const uidValidity = await this.getUidValidity(client, sentMailbox);

      const externalId = uidValidity
        ? `${sentFolder.path}:${uidValidity.toString()}`
@@ -88,6 +93,13 @@ export class ImapGetAllFoldersService implements MessageFolderDriver {
    }

    for (const mailbox of mailboxList) {
+      if (!this.isValidMailbox(mailbox, folders)) {
+        if (!pathToExternalIdMap.has(mailbox.path)) {
+          pathToExternalIdMap.set(mailbox.path, mailbox.path);
+        }
+        continue;
+      }
+
      const uidValidity = await this.getUidValidity(client, mailbox);
      const externalId = uidValidity
        ? `${mailbox.path}:${uidValidity}`
@@ -95,25 +107,23 @@ export class ImapGetAllFoldersService implements MessageFolderDriver {

      pathToExternalIdMap.set(mailbox.path, externalId);

-      if (this.isValidMailbox(mailbox, folders)) {
-        const standardFolder = getStandardFolderByRegex(mailbox.name);
+      const standardFolder = getStandardFolderByRegex(mailbox.name);

-        if (!shouldCreateFolderByDefault(standardFolder)) {
-          continue;
-        }
-
-        const isSynced = shouldSyncFolderByDefault(
-          messageChannel.messageFolderImportPolicy,
-        );
-
-        folders.push({
-          externalId,
-          name: mailbox.name,
-          isSynced,
-          isSentFolder: false,
-          parentFolderId: mailbox.parentPath || null,
-        });
+      if (!shouldCreateFolderByDefault(standardFolder)) {
+        continue;
      }
+
+      const isSynced = shouldSyncFolderByDefault(
+        messageChannel.messageFolderImportPolicy,
+      );
+
+      folders.push({
+        externalId,
+        name: mailbox.name,
+        isSynced,
+        isSentFolder: false,
+        parentFolderId: mailbox.parentPath || null,
+      });
    }

    for (const folder of folders) {
@@ -127,11 +137,15 @@ export class ImapGetAllFoldersService implements MessageFolderDriver {
    return folders;
  }

+  private isMailboxSelectable(mailbox: ListResponse): boolean {
+    return !mailbox.flags?.has('\\Noselect');
+  }
+
  private isValidMailbox(
    mailbox: ListResponse,
    existingFolders: DiscoveredMessageFolder[],
  ): boolean {
-    if (mailbox.flags?.has('\\Noselect')) {
+    if (!this.isMailboxSelectable(mailbox)) {
      return false;
    }