fix: use IMAP folder path from externalId for sent message append

https://sonarly.com/issue/34327?type=bug Emails sent via SMTP from the UI show "Command failed" error because the IMAP APPEND command receives the folder's leaf name instead of its full IMAP path, causing the server to reject the save-to-Sent operation on providers with hierarchical folder structures. Fix: Fixed two issues in `ImapSmtpMessageOutboundService.sendMessage`: 1. **Wrong folder identifier for IMAP APPEND**: Changed from using `sentFolder.name` (leaf folder name like "Sent") to extracting the full IMAP mailbox path from `sentFolder.externalId` (format `<path>:<uidValidity>`, e.g., "INBOX.Sent:12345" → "INBOX.Sent"). This matches how the rest of the codebase resolves IMAP paths from `externalId` (see `imap-get-all-folders.service.ts:167`). Falls back to `sentFolder.name` if `externalId` is null. 2. **Missing error handling on IMAP append**: The IMAP `append()` call to save a copy in the Sent folder is now wrapped in try/catch. Since the email is already delivered via SMTP at this point, a failure to save to the Sent folder should not cause the entire send operation to return an error. The failure is logged as a warning (following the pattern from `send-email.service.ts`). The next sync cycle will pick up the sent message anyway. Added a `getSentFolderPath` private method for clean path extraction and a unit test file with 4 test cases covering: correct path extraction from externalId, fallback to name, graceful append failure handling, and skip behavior when no sent folder exists.
2026-05-05 07:35:35 +00:00
parent df63dbff05
commit 285d6cf989
2 changed files with 197 additions and 3 deletions
@@ -0,0 +1,170 @@
+import { Test, type TestingModule } from '@nestjs/testing';
+import { getRepositoryToken } from '@nestjs/typeorm';
+
+import { ConnectedAccountProvider } from 'twenty-shared/types';
+
+import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
+import { MessageFolderEntity } from 'src/engine/metadata-modules/message-folder/entities/message-folder.entity';
+import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { ImapClientProvider } from 'src/modules/messaging/message-import-manager/drivers/imap/providers/imap-client.provider';
+import { ImapFindDraftsFolderService } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-find-drafts-folder.service';
+import { SmtpClientProvider } from 'src/modules/messaging/message-import-manager/drivers/smtp/providers/smtp-client.provider';
+import { ImapSmtpMessageOutboundService } from 'src/modules/messaging/message-outbound-manager/drivers/imap/services/imap-smtp-message-outbound.service';
+
+jest.mock('nodemailer/lib/mail-composer', () => {
+  return jest.fn().mockImplementation(() => ({
+    compile: jest.fn().mockReturnValue({
+      build: jest
+        .fn()
+        .mockResolvedValue(
+          Buffer.from(
+            'Message-ID: <test-message-id@example.com>\r\nContent: test',
+          ),
+        ),
+    }),
+  }));
+});
+
+describe('ImapSmtpMessageOutboundService', () => {
+  let service: ImapSmtpMessageOutboundService;
+
+  const mockSmtpSendMail = jest.fn().mockResolvedValue({});
+  const mockImapAppend = jest.fn().mockResolvedValue(undefined);
+  const mockImapClient = { append: mockImapAppend };
+
+  const connectedAccount = {
+    id: 'account-1',
+    handle: 'user@example.com',
+    provider: ConnectedAccountProvider.IMAP_SMTP_CALDAV,
+    connectionParameters: {
+      IMAP: { host: 'imap.example.com', port: 993 },
+      SMTP: { host: 'smtp.example.com', port: 587 },
+    },
+  } as unknown as ConnectedAccountEntity;
+
+  const messageChannel = {
+    id: 'channel-1',
+    connectedAccountId: 'account-1',
+    handle: 'user@example.com',
+  };
+
+  beforeEach(async () => {
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        ImapSmtpMessageOutboundService,
+        {
+          provide: SmtpClientProvider,
+          useValue: {
+            getSmtpClient: jest
+              .fn()
+              .mockResolvedValue({ sendMail: mockSmtpSendMail }),
+          },
+        },
+        {
+          provide: ImapClientProvider,
+          useValue: {
+            getClient: jest.fn().mockResolvedValue(mockImapClient),
+            closeClient: jest.fn().mockResolvedValue(undefined),
+          },
+        },
+        {
+          provide: ImapFindDraftsFolderService,
+          useValue: {},
+        },
+        {
+          provide: getRepositoryToken(MessageChannelEntity),
+          useValue: {
+            findOne: jest.fn().mockResolvedValue(messageChannel),
+          },
+        },
+        {
+          provide: getRepositoryToken(MessageFolderEntity),
+          useValue: {
+            findOne: jest.fn(),
+          },
+        },
+      ],
+    }).compile();
+
+    service = module.get(ImapSmtpMessageOutboundService);
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should use full path from externalId for IMAP append', async () => {
+    const messageFolderRepo = service['messageFolderRepository'];
+
+    (messageFolderRepo.findOne as jest.Mock).mockResolvedValue({
+      id: 'folder-1',
+      name: 'Sent',
+      externalId: 'INBOX.Sent:12345',
+      isSentFolder: true,
+    });
+
+    await service.sendMessage(
+      { to: 'recipient@example.com', subject: 'Test', body: 'Hello' },
+      connectedAccount,
+    );
+
+    expect(mockImapAppend).toHaveBeenCalledWith(
+      'INBOX.Sent',
+      expect.any(Buffer),
+    );
+  });
+
+  it('should fall back to name when externalId is null', async () => {
+    const messageFolderRepo = service['messageFolderRepository'];
+
+    (messageFolderRepo.findOne as jest.Mock).mockResolvedValue({
+      id: 'folder-1',
+      name: 'Sent',
+      externalId: null,
+      isSentFolder: true,
+    });
+
+    await service.sendMessage(
+      { to: 'recipient@example.com', subject: 'Test', body: 'Hello' },
+      connectedAccount,
+    );
+
+    expect(mockImapAppend).toHaveBeenCalledWith('Sent', expect.any(Buffer));
+  });
+
+  it('should not fail the send when IMAP append throws', async () => {
+    const messageFolderRepo = service['messageFolderRepository'];
+
+    (messageFolderRepo.findOne as jest.Mock).mockResolvedValue({
+      id: 'folder-1',
+      name: 'Sent',
+      externalId: 'Sent:12345',
+      isSentFolder: true,
+    });
+
+    mockImapAppend.mockRejectedValueOnce(new Error('Command failed'));
+
+    const result = await service.sendMessage(
+      { to: 'recipient@example.com', subject: 'Test', body: 'Hello' },
+      connectedAccount,
+    );
+
+    expect(result).toBeDefined();
+    expect(result.headerMessageId).toBeDefined();
+    expect(mockSmtpSendMail).toHaveBeenCalled();
+  });
+
+  it('should skip IMAP append when no sent folder is found', async () => {
+    const messageFolderRepo = service['messageFolderRepository'];
+
+    (messageFolderRepo.findOne as jest.Mock).mockResolvedValue(null);
+
+    const result = await service.sendMessage(
+      { to: 'recipient@example.com', subject: 'Test', body: 'Hello' },
+      connectedAccount,
+    );
+
+    expect(result).toBeDefined();
+    expect(mockImapAppend).not.toHaveBeenCalled();
+  });
+});
@@ -1,4 +1,4 @@
-import { Injectable } from '@nestjs/common';
+import { Injectable, Logger } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';

 import MailComposer from 'nodemailer/lib/mail-composer';
@@ -20,6 +20,8 @@ import { toMailComposerOptions } from 'src/modules/messaging/message-outbound-ma

@Injectable()
 export class ImapSmtpMessageOutboundService implements MessageOutboundDriver {
+  private readonly logger = new Logger(ImapSmtpMessageOutboundService.name);
+
  constructor(
    private readonly smtpClientProvider: SmtpClientProvider,
    private readonly imapClientProvider: ImapClientProvider,
@@ -76,8 +78,16 @@ export class ImapSmtpMessageOutboundService implements MessageOutboundDriver {
        });
      }

-      if (isDefined(sentFolder) && isDefined(sentFolder.name)) {
-        await imapClient.append(sentFolder.name, messageBuffer);
+      const sentFolderPath = this.getSentFolderPath(sentFolder);
+
+      if (isDefined(sentFolderPath)) {
+        try {
+          await imapClient.append(sentFolderPath, messageBuffer);
+        } catch (error) {
+          this.logger.warn(
+            `Failed to append message to sent folder "${sentFolderPath}": ${error}`,
+          );
+        }
      }

      await this.imapClientProvider.closeClient(imapClient);
@@ -125,6 +135,20 @@ export class ImapSmtpMessageOutboundService implements MessageOutboundDriver {
    }
  }

+  private getSentFolderPath(
+    sentFolder: MessageFolderEntity | null,
+  ): string | null {
+    if (!isDefined(sentFolder)) {
+      return null;
+    }
+
+    if (isDefined(sentFolder.externalId)) {
+      return sentFolder.externalId.split(':')[0];
+    }
+
+    return sentFolder.name;
+  }
+
  private async compileRawMessage(
    from: string,
    sendMessageInput: SendMessageInput,