fix: address review findings for email forwarding feature

- P0: Move S3 object to failed/ on download error (prevents infinite retry)
- P1: Return existing channel if user already has one (idempotent creation)
- P1: Map EMAIL_FORWARDING_NOT_CONFIGURED to InternalServerError
- P1: Remove redundant MessageChannelType guard in hasPendingConfiguration
- P1: Reset modal forwardingAddress state on close
- Update test to verify moveToFailed on download error

https://claude.ai/code/session_01KpyF6p4cUEnuaT4h8DP5Pm

packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailForwardingModal.tsx

@@ -65,10 +65,12 @@ const StyledButtonContainer = styled.div`
 
 type SettingsAccountsEmailForwardingModalProps = {
   forwardingAddress: string;
+  onClose?: () => void;
 };
 
 export const SettingsAccountsEmailForwardingModal = ({
   forwardingAddress,
+  onClose,
 }: SettingsAccountsEmailForwardingModalProps) => {
   const { t } = useLingui();
   const { copyToClipboard } = useCopyToClipboard();
@@ -114,7 +116,10 @@ export const SettingsAccountsEmailForwardingModal = ({
             title={t`Done`}
             variant="primary"
             size="small"
-            onClick={() => closeModal(EMAIL_FORWARDING_MODAL_ID)}
+            onClick={() => {
+              closeModal(EMAIL_FORWARDING_MODAL_ID);
+              onClose?.();
+            }}
           />
         </StyledButtonContainer>
       </StyledModalContent>

packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsListEmptyStateCard.tsx

@@ -120,6 +120,7 @@ export const SettingsAccountsListEmptyStateCard = () => {
       {forwardingAddress && (
         <SettingsAccountsEmailForwardingModal
           forwardingAddress={forwardingAddress}
+          onClose={() => setForwardingAddress(null)}
         />
       )}
     </>

packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsRowDropdownMenu.tsx

@@ -4,7 +4,6 @@ import {
   CalendarChannelSyncStage,
   ConnectedAccountProvider,
   MessageChannelSyncStage,
-  MessageChannelType,
   SettingsPath,
 } from 'twenty-shared/types';
 
@@ -61,7 +60,6 @@ export const SettingsAccountsRowDropdownMenu = ({
     !isEmailForwarding &&
     (account.messageChannels.some(
       (channel) =>
-        channel.type !== MessageChannelType.EMAIL_FORWARDING &&
         channel.syncStage === MessageChannelSyncStage.PENDING_CONFIGURATION,
     ) ||
       account.calendarChannels.some(

packages/twenty-server/src/engine/metadata-modules/message-channel/message-channel-metadata.service.ts

@@ -214,6 +214,25 @@ export class MessageChannelMetadataService {
       );
     }
 
+    const existingChannel = await this.repository.findOne({
+      where: {
+        workspaceId,
+        type: MessageChannelType.EMAIL_FORWARDING,
+        connectedAccountId: In(
+          await this.connectedAccountMetadataService
+            .getUserConnectedAccountIds({ userWorkspaceId, workspaceId })
+            .then((ids) => (ids.length > 0 ? ids : ['__none__'])),
+        ),
+      },
+    });
+
+    if (existingChannel) {
+      return {
+        messageChannel: existingChannel,
+        forwardingAddress: existingChannel.handle,
+      };
+    }
+
     const localPart =
       INBOUND_EMAIL_LOCAL_PART_PREFIX +
       randomBytes(INBOUND_EMAIL_LOCAL_PART_RANDOM_BYTES).toString('hex');

packages/twenty-server/src/engine/metadata-modules/message-channel/utils/message-channel-graphql-api-exception-handler.util.ts

@@ -2,6 +2,7 @@ import { assertUnreachable } from 'twenty-shared/utils';
 
 import {
   ForbiddenError,
+  InternalServerError,
   NotFoundError,
   UserInputError,
 } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
@@ -24,7 +25,7 @@ export const messageChannelGraphqlApiExceptionHandler = (error: Error) => {
       case MessageChannelExceptionCode.MESSAGE_CHANNEL_OWNERSHIP_VIOLATION:
         throw new ForbiddenError(error);
       case MessageChannelExceptionCode.EMAIL_FORWARDING_NOT_CONFIGURED:
-        throw new UserInputError(error);
+        throw new InternalServerError(error);
       default: {
         return assertUnreachable(error.code);
       }

packages/twenty-server/src/modules/messaging/message-import-manager/crons/jobs/messaging-messages-import.cron.job.ts

@@ -4,7 +4,10 @@ import { isDefined } from 'twenty-shared/utils';
 import { WorkspaceActivationStatus } from 'twenty-shared/workspace';
 import { DataSource, Repository } from 'typeorm';
 
-import { MessageChannelSyncStage, MessageChannelType } from 'twenty-shared/types';
+import {
+  MessageChannelSyncStage,
+  MessageChannelType,
+} from 'twenty-shared/types';
 import { SentryCronMonitor } from 'src/engine/core-modules/cron/sentry-cron-monitor.decorator';
 import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service';
 import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator';

packages/twenty-server/src/modules/messaging/message-import-manager/drivers/inbound-email/services/__tests__/inbound-email-import.service.spec.ts

@@ -177,7 +177,7 @@ describe('InboundEmailImportService', () => {
     expect(storageService.moveToProcessed).toHaveBeenCalledWith(TEST_S3_KEY);
   });
 
-  it('should return "parse_failed" when download fails', async () => {
+  it('should return "parse_failed" and move to failed/ when download fails', async () => {
     storageService.getRawMessage.mockRejectedValue(
       new Error('S3 download error'),
     );
@@ -188,6 +188,7 @@ describe('InboundEmailImportService', () => {
       kind: 'parse_failed',
       error: 'S3 download error',
     });
+    expect(storageService.moveToFailed).toHaveBeenCalledWith(TEST_S3_KEY);
   });
 
   it('should return "parse_failed" when parsing fails', async () => {

packages/twenty-server/src/modules/messaging/message-import-manager/drivers/inbound-email/services/inbound-email-import.service.ts

@@ -61,6 +61,7 @@ export class InboundEmailImportService {
       this.logger.error(
         `Failed to download inbound email from S3 key ${s3Key}: ${message}`,
       );
+      await this.safeMove(s3Key, 'failed');
 
       return { kind: 'parse_failed', error: message };
     }