fix(twenty-server): compose email from the caller's own connected account (#23793)

### The bug `draft_email` and `send_email` take an optional `connectedAccountId`. When an agent omits it — which it does whenever it has no way to know the id — `EmailComposerService` resolved the account like this: ```ts const allAccounts = await this.connectedAccountRepository.find({ where: { workspaceId, archivedAt: IsNull() }, }); return allAccounts[0].id; ``` The first connected account **in the workspace**, ignoring the `userWorkspaceId` that `ToolExecutionContext` already carries — with no `ORDER BY`, so "first" is whatever the planner returns. We hit this on our own workspace: an agent chat drafted a customer email on behalf of one user, and the draft landed in a different user's mailbox. The tool reported `success: true` with a `connectedAccountId` belonging to someone who was not in the conversation, so nothing surfaced the mistake. `send_email` shares this composer, so the same fallback sends mail from another person's address. ### The fix - **No id supplied** → the caller's own account (`context.userWorkspaceId`), else an account whose `visibility` is `workspace`, else throw `CONNECTED_ACCOUNT_NOT_FOUND`. Never a colleague's private mailbox by accident. - **Id supplied** → used as given, whoever owns it. Blocking a member from composing through another member's account is a product decision this PR does not make; the mix-up above happens when no id is passed at all. - **No `userWorkspaceId`** (workflow run) → unchanged. Ordering is `createdAt ASC, id ASC` so the no-caller path is deterministic when rows share a `createdAt` — which the seed data does. ### Verified against a real workspace Run locally against the seeded `test` database — 7 connected accounts in one workspace, owned by four different members, **all sharing one `createdAt`**. Same spec, composer swapped: | Scenario | on `main` | with this PR | |---|---|---| | Phil's agent composes, no id | **tim@apple.dev's account** | phil's own | | explicit id (jony's), caller is phil | jony's | jony's | | workflow run (no caller), explicit id | jony's | jony's | | **workflow run (no caller), no id** | **first account, unordered** | **first account, `createdAt`/`id` ordered** | | caller with no account | silently resolved a colleague's | throws | ### What this does not fix A workflow run carries no caller: `ToolBackedWorkflowAction` executes the tool with `{ workspaceId }` and no `userWorkspaceId`. So when an email step's sender resolves to nothing — `postprocessInput` guards for it — the composer still falls back to the workspace's first account, because there is no identity to attribute the mail to. The pick is at least deterministic now. Giving workflow runs an owner is a separate change. Normal workflow steps are unaffected: `EmailWorkflowActionBase.resolveSenderConnectedAccountId` resolves the configured sender (a connected-account id, or a workspace member id from a resolved variable) and passes it explicitly. ### Behaviour change to expect A caller with no connected account of their own, in a workspace with no shared account, now gets an error where the call previously "succeeded" from a colleague's mailbox. ### Tests Resolution is exercised by `test/integration/email-tool/suites/email-composer-connected-account.integration-spec.ts` against a real workspace — eight cases: supplied id honoured, supplied id with no caller, invalid id, unknown id, caller's own account, workspace-shared fallback (flips `visibility` in Postgres and restores it), no usable account, and first-account-when-no-caller. The service's unit spec is deleted: mocking the DI graph asserted the mock rather than the resolution, and every case it covered now runs against the database. The pure selection logic keeps unit specs — `select-connected-account-id-for-caller.util.spec.ts` and `is-connected-account-usable-by-caller.util.spec.ts`. Not covered here: the workflow chain itself (`postprocessInput` → `resolveSenderConnectedAccountId` → `DraftEmailWorkflowAction`), which this PR does not change. `npx nx typecheck twenty-server`, the email-tool and connected-account suites, and the integration spec all pass; oxlint type-aware clean.
2026-08-05 11:31:46 +02:00
parent 6d83018b6f
commit 13a2e3ebe8
8 changed files with 351 additions and 111 deletions
@@ -1,94 +0,0 @@
-import { ConnectedAccountProvider } from 'twenty-shared/types';
-
-import { EmailComposerService } from 'src/engine/core-modules/tool/tools/email-tool/email-composer.service';
-
-const WORKSPACE_ID = '20202020-0000-4000-8000-000000000000';
-const CONNECTED_ACCOUNT_ID = '20202020-1111-4111-8111-111111111111';
-
-const buildAccount = (id: string) => ({
-  id,
-  handle: 'tim@apple.dev',
-  provider: ConnectedAccountProvider.GOOGLE,
-  scopes: ['email'],
-  connectionParameters: null,
-  messageChannels: [{ id: 'message-channel-1', handle: 'tim@apple.dev' }],
-});
-
-const baseParams = {
-  recipients: { to: 'test@example.com' },
-  subject: 'Subject',
-  body: '<p>body</p>',
-  files: [],
-};
-
-const context = { workspaceId: WORKSPACE_ID };
-
-describe('EmailComposerService connected account resolution', () => {
-  let service: EmailComposerService;
-  let connectedAccountRepository: {
-    findOne: jest.Mock;
-    find: jest.Mock;
-  };
-  let globalWorkspaceOrmManager: {
-    executeInWorkspaceContext: jest.Mock;
-    getRepository: jest.Mock;
-  };
-
-  beforeEach(() => {
-    jest.clearAllMocks();
-
-    connectedAccountRepository = { findOne: jest.fn(), find: jest.fn() };
-    globalWorkspaceOrmManager = {
-      executeInWorkspaceContext: jest.fn((callback) => callback()),
-      getRepository: jest.fn(),
-    };
-
-    service = new EmailComposerService(
-      globalWorkspaceOrmManager as never,
-      connectedAccountRepository as never,
-      { find: jest.fn() } as never,
-      {} as never,
-    );
-  });
-
-  it('uses the connected account matching the provided id', async () => {
-    connectedAccountRepository.findOne.mockResolvedValue(
-      buildAccount(CONNECTED_ACCOUNT_ID),
-    );
-
-    const result = await service.composeEmail(
-      { ...baseParams, connectedAccountId: CONNECTED_ACCOUNT_ID },
-      context,
-    );
-
-    expect(result.success).toBe(true);
-    expect(result.success && result.data.connectedAccount.id).toBe(
-      CONNECTED_ACCOUNT_ID,
-    );
-    expect(connectedAccountRepository.findOne).toHaveBeenCalledWith(
-      expect.objectContaining({
-        where: { id: CONNECTED_ACCOUNT_ID, workspaceId: WORKSPACE_ID },
-      }),
-    );
-  });
-
-  it('throws when the id is not a valid UUID', async () => {
-    await expect(
-      service.composeEmail(
-        { ...baseParams, connectedAccountId: 'not-a-uuid' },
-        context,
-      ),
-    ).rejects.toThrow('Connected account id is not a valid UUID');
-  });
-
-  it('throws when no connected account matches the provided id', async () => {
-    connectedAccountRepository.findOne.mockResolvedValue(null);
-
-    await expect(
-      service.composeEmail(
-        { ...baseParams, connectedAccountId: CONNECTED_ACCOUNT_ID },
-        context,
-      ),
-    ).rejects.toThrow(`No connected account found for id`);
-  });
-});
@@ -9,7 +9,7 @@ import {
  ConnectedAccountProvider,
  type EmailAttachment,
 } from 'twenty-shared/types';
-import { isDefined, isValidUuid } from 'twenty-shared/utils';
+import { isDefined, isNonEmptyArray, isValidUuid } from 'twenty-shared/utils';
 import { In, IsNull, LessThanOrEqual, type Repository } from 'typeorm';
 import { z } from 'zod';

@@ -23,6 +23,7 @@ import {
 import { type ComposeEmailParams } from 'src/engine/core-modules/tool/tools/email-tool/types/compose-email-params.type';
 import { EmailComposerResult } from 'src/engine/core-modules/tool/tools/email-tool/types/email-composer-result.type';
 import { parseCommaSeparatedEmails } from 'src/engine/core-modules/tool/tools/email-tool/utils/parse-comma-separated-emails.util';
+import { selectConnectedAccountIdForCaller } from 'src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util';
 import { type ToolExecutionContext } from 'src/engine/core-modules/tool/types/tool-execution-context.type';
 import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
 import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
@@ -52,10 +53,13 @@ export class EmailComposerService {
    private readonly fileService: FileService,
  ) {}

-  private async getConnectedAccountOrThrow(
-    connectedAccountId: string,
-    workspaceId: string,
-  ): Promise<ConnectedAccountEntity> {
+  private async getConnectedAccountOrThrow({
+    connectedAccountId,
+    workspaceId,
+  }: {
+    connectedAccountId: string;
+    workspaceId: string;
+  }): Promise<ConnectedAccountEntity> {
    if (!isValidUuid(connectedAccountId)) {
      throw new EmailToolException(
        `Connected account id is not a valid UUID`,
@@ -89,25 +93,46 @@ export class EmailComposerService {
    );
  }

-  private async getOrThrowFirstConnectedAccountId(
-    workspaceId: string,
-  ): Promise<string> {
+  private async getDefaultConnectedAccountIdOrThrow({
+    workspaceId,
+    userWorkspaceId,
+  }: {
+    workspaceId: string;
+    userWorkspaceId?: string;
+  }): Promise<string> {
    const authContext = buildSystemAuthContext(workspaceId);

    return this.globalWorkspaceOrmManager.executeInWorkspaceContext(
      async () => {
        const allAccounts = await this.connectedAccountRepository.find({
          where: { workspaceId, archivedAt: IsNull() },
+          order: { createdAt: 'ASC', id: 'ASC' },
        });

-        if (!allAccounts || allAccounts.length === 0) {
+        if (!isNonEmptyArray(allAccounts)) {
          throw new EmailToolException(
            'No connected accounts found for this workspace',
            EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
          );
        }

-        return allAccounts[0].id;
+        if (!isDefined(userWorkspaceId)) {
+          return allAccounts[0].id;
+        }
+
+        const connectedAccountId = selectConnectedAccountIdForCaller({
+          connectedAccounts: allAccounts,
+          userWorkspaceId,
+        });
+
+        if (!isDefined(connectedAccountId)) {
+          throw new EmailToolException(
+            `No connected account available for user workspace '${userWorkspaceId}'`,
+            EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
+          );
+        }
+
+        return connectedAccountId;
      },
      authContext,
    );
@@ -312,7 +337,7 @@ export class EmailComposerService {
    parameters: ComposeEmailParams,
    context: ToolExecutionContext,
  ): Promise<EmailComposerResult> {
-    const { workspaceId } = context;
+    const { workspaceId, userWorkspaceId } = context;
    const { subject, body, files, inReplyTo } = parameters;
    let { connectedAccountId } = parameters;

@@ -351,14 +376,16 @@ export class EmailComposerService {
    const toRecipientsDisplay = recipients.to.join(', ');

    if (!connectedAccountId) {
-      connectedAccountId =
-        await this.getOrThrowFirstConnectedAccountId(workspaceId);
+      connectedAccountId = await this.getDefaultConnectedAccountIdOrThrow({
+        workspaceId,
+        userWorkspaceId,
+      });
    }

-    const connectedAccount = await this.getConnectedAccountOrThrow(
+    const connectedAccount = await this.getConnectedAccountOrThrow({
      connectedAccountId,
      workspaceId,
-    );
+    });

    const messageChannel =
      connectedAccount.provider === ConnectedAccountProvider.EMAIL_GROUP
@@ -0,0 +1,69 @@
+import { selectConnectedAccountIdForCaller } from 'src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util';
+
+const USER_WORKSPACE_ID = '20202020-2222-4222-8222-222222222222';
+const OTHER_USER_WORKSPACE_ID = '20202020-3333-4333-8333-333333333333';
+
+const ownAccount = {
+  id: 'own-account-id',
+  userWorkspaceId: USER_WORKSPACE_ID,
+  visibility: 'user' as const,
+};
+
+const colleagueAccount = {
+  id: 'colleague-account-id',
+  userWorkspaceId: OTHER_USER_WORKSPACE_ID,
+  visibility: 'user' as const,
+};
+
+const sharedAccount = {
+  id: 'shared-account-id',
+  userWorkspaceId: OTHER_USER_WORKSPACE_ID,
+  visibility: 'workspace' as const,
+};
+
+describe('selectConnectedAccountIdForCaller', () => {
+  it("returns the caller's own account even when another comes first", () => {
+    expect(
+      selectConnectedAccountIdForCaller({
+        connectedAccounts: [colleagueAccount, ownAccount],
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe('own-account-id');
+  });
+
+  it("prefers the caller's own account over a shared one", () => {
+    expect(
+      selectConnectedAccountIdForCaller({
+        connectedAccounts: [sharedAccount, ownAccount],
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe('own-account-id');
+  });
+
+  it('falls back to an account shared with the whole workspace', () => {
+    expect(
+      selectConnectedAccountIdForCaller({
+        connectedAccounts: [colleagueAccount, sharedAccount],
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe('shared-account-id');
+  });
+
+  it('returns undefined rather than a colleague account', () => {
+    expect(
+      selectConnectedAccountIdForCaller({
+        connectedAccounts: [colleagueAccount],
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBeUndefined();
+  });
+
+  it('returns undefined when there is no account at all', () => {
+    expect(
+      selectConnectedAccountIdForCaller({
+        connectedAccounts: [],
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBeUndefined();
+  });
+});
@@ -0,0 +1,25 @@
+import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util';
+import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+
+export const selectConnectedAccountIdForCaller = ({
+  connectedAccounts,
+  userWorkspaceId,
+}: {
+  connectedAccounts: Pick<
+    ConnectedAccountEntity,
+    'id' | 'visibility' | 'userWorkspaceId'
+  >[];
+  userWorkspaceId: string;
+}): string | undefined => {
+  const ownAccount = connectedAccounts.find(
+    (connectedAccount) => connectedAccount.userWorkspaceId === userWorkspaceId,
+  );
+
+  const usableAccount =
+    ownAccount ??
+    connectedAccounts.find((connectedAccount) =>
+      isConnectedAccountUsableByCaller({ connectedAccount, userWorkspaceId }),
+    );
+
+  return usableAccount?.id;
+};
@@ -17,6 +17,7 @@ import {
 } from 'src/engine/metadata-modules/connected-account/connected-account.exception';
 import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
 import { type ConnectedAccountDeletedEvent } from 'src/engine/metadata-modules/connected-account/types/connected-account-deleted.type';
+import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util';
 import { MESSAGE_CHANNEL_DELETED_EVENT } from 'src/engine/metadata-modules/message-channel/constants/message-channel-deleted.constant';
 import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
 import { type MessageChannelDeletedEvent } from 'src/engine/metadata-modules/message-channel/types/message-channel-deleted.type';
@@ -95,8 +96,7 @@ export class ConnectedAccountMetadataService {
    }

    if (
-      connectedAccount.visibility !== 'workspace' &&
-      connectedAccount.userWorkspaceId !== userWorkspaceId
+      !isConnectedAccountUsableByCaller({ connectedAccount, userWorkspaceId })
    ) {
      throw new ConnectedAccountException(
        `Connected account ${id} does not belong to user workspace ${userWorkspaceId}`,
@@ -0,0 +1,42 @@
+import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util';
+
+const USER_WORKSPACE_ID = '20202020-2222-4222-8222-222222222222';
+const OTHER_USER_WORKSPACE_ID = '20202020-3333-4333-8333-333333333333';
+
+describe('isConnectedAccountUsableByCaller', () => {
+  it('accepts an account the caller owns', () => {
+    expect(
+      isConnectedAccountUsableByCaller({
+        connectedAccount: {
+          userWorkspaceId: USER_WORKSPACE_ID,
+          visibility: 'user',
+        },
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe(true);
+  });
+
+  it('accepts an account shared with the whole workspace', () => {
+    expect(
+      isConnectedAccountUsableByCaller({
+        connectedAccount: {
+          userWorkspaceId: OTHER_USER_WORKSPACE_ID,
+          visibility: 'workspace',
+        },
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe(true);
+  });
+
+  it('rejects another user private account', () => {
+    expect(
+      isConnectedAccountUsableByCaller({
+        connectedAccount: {
+          userWorkspaceId: OTHER_USER_WORKSPACE_ID,
+          visibility: 'user',
+        },
+        userWorkspaceId: USER_WORKSPACE_ID,
+      }),
+    ).toBe(false);
+  });
+});
@@ -0,0 +1,14 @@
+import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+
+export const isConnectedAccountUsableByCaller = ({
+  connectedAccount,
+  userWorkspaceId,
+}: {
+  connectedAccount: Pick<
+    ConnectedAccountEntity,
+    'visibility' | 'userWorkspaceId'
+  >;
+  userWorkspaceId: string;
+}): boolean =>
+  connectedAccount.visibility === 'workspace' ||
+  connectedAccount.userWorkspaceId === userWorkspaceId;
@@ -0,0 +1,157 @@
+import { EmailComposerService } from 'src/engine/core-modules/tool/tools/email-tool/email-composer.service';
+
+import { getAppProviderByClassName } from 'test/integration/utils/get-app-provider-by-class-name.util';
+
+const WORKSPACE_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419';
+
+const PHIL_USER_WORKSPACE_ID = '20202020-7169-42cf-bc47-1cfef15264b1';
+const PHIL_CONNECTED_ACCOUNT_ID = '20202020-cafc-4323-908d-e5b42ad69fdf';
+
+const JONY_CONNECTED_ACCOUNT_ID = '20202020-0cc8-4d60-a3a4-803245698908';
+
+const UNKNOWN_USER_WORKSPACE_ID = '20202020-0000-4000-8000-00000000dead';
+const UNKNOWN_CONNECTED_ACCOUNT_ID = '20202020-0000-4000-8000-00000000beef';
+
+const baseParams = {
+  recipients: { to: 'customer@example.com' },
+  subject: 'Subject',
+  body: '<p>body</p>',
+  files: [],
+};
+
+const getFirstWorkspaceConnectedAccountId = async (): Promise<string> => {
+  const [{ id }] = await global.testDataSource.query(
+    `SELECT id FROM core."connectedAccount"
+     WHERE "workspaceId" = $1 AND "archivedAt" IS NULL
+     ORDER BY "createdAt" ASC, id ASC
+     LIMIT 1`,
+    [WORKSPACE_ID],
+  );
+
+  return id;
+};
+
+const setVisibility = async (
+  connectedAccountId: string,
+  visibility: 'user' | 'workspace',
+) => {
+  await global.testDataSource.query(
+    `UPDATE core."connectedAccount" SET visibility = $1 WHERE id = $2`,
+    [visibility, connectedAccountId],
+  );
+};
+
+describe('EmailComposerService connected account resolution (integration)', () => {
+  let service: EmailComposerService;
+
+  beforeAll(() => {
+    service = getAppProviderByClassName<EmailComposerService>(
+      'EmailComposerService',
+    );
+  });
+
+  describe('when the caller names a connected account', () => {
+    it('uses that account, whoever owns it', async () => {
+      const result = await service.composeEmail(
+        { ...baseParams, connectedAccountId: JONY_CONNECTED_ACCOUNT_ID },
+        { workspaceId: WORKSPACE_ID, userWorkspaceId: PHIL_USER_WORKSPACE_ID },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.success && result.data.connectedAccount.id).toBe(
+        JONY_CONNECTED_ACCOUNT_ID,
+      );
+    });
+
+    it('uses that account when there is no caller (workflow run)', async () => {
+      const result = await service.composeEmail(
+        { ...baseParams, connectedAccountId: JONY_CONNECTED_ACCOUNT_ID },
+        { workspaceId: WORKSPACE_ID },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.success && result.data.connectedAccount.id).toBe(
+        JONY_CONNECTED_ACCOUNT_ID,
+      );
+    });
+
+    it('throws when the id is not a valid UUID', async () => {
+      await expect(
+        service.composeEmail(
+          { ...baseParams, connectedAccountId: 'not-a-uuid' },
+          {
+            workspaceId: WORKSPACE_ID,
+            userWorkspaceId: PHIL_USER_WORKSPACE_ID,
+          },
+        ),
+      ).rejects.toThrow('Connected account id is not a valid UUID');
+    });
+
+    it('throws when no connected account matches the id', async () => {
+      await expect(
+        service.composeEmail(
+          { ...baseParams, connectedAccountId: UNKNOWN_CONNECTED_ACCOUNT_ID },
+          {
+            workspaceId: WORKSPACE_ID,
+            userWorkspaceId: PHIL_USER_WORKSPACE_ID,
+          },
+        ),
+      ).rejects.toThrow('No connected account found for id');
+    });
+  });
+
+  describe('when the caller names none', () => {
+    it('composes from the caller own account rather than the first of the workspace', async () => {
+      const result = await service.composeEmail(baseParams, {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: PHIL_USER_WORKSPACE_ID,
+      });
+
+      expect(result.success).toBe(true);
+      expect(result.success && result.data.connectedAccount.id).toBe(
+        PHIL_CONNECTED_ACCOUNT_ID,
+      );
+    });
+
+    it('falls back to an account shared with the whole workspace', async () => {
+      await setVisibility(JONY_CONNECTED_ACCOUNT_ID, 'workspace');
+
+      try {
+        const result = await service.composeEmail(baseParams, {
+          workspaceId: WORKSPACE_ID,
+          userWorkspaceId: UNKNOWN_USER_WORKSPACE_ID,
+        });
+
+        expect(result.success).toBe(true);
+        expect(result.success && result.data.connectedAccount.id).toBe(
+          JONY_CONNECTED_ACCOUNT_ID,
+        );
+      } finally {
+        await setVisibility(JONY_CONNECTED_ACCOUNT_ID, 'user');
+      }
+    });
+
+    it('throws rather than composing from a colleague account', async () => {
+      await expect(
+        service.composeEmail(baseParams, {
+          workspaceId: WORKSPACE_ID,
+          userWorkspaceId: UNKNOWN_USER_WORKSPACE_ID,
+        }),
+      ).rejects.toThrow('No connected account available for user workspace');
+    });
+
+    it('takes the first workspace account when there is no caller (workflow run)', async () => {
+      const firstWorkspaceConnectedAccountId =
+        await getFirstWorkspaceConnectedAccountId();
+
+      const result = await service.composeEmail(baseParams, {
+        workspaceId: WORKSPACE_ID,
+      });
+
+      expect(result.success).toBe(true);
+      expect(result.success && result.data.connectedAccount.id).toBe(
+        firstWorkspaceConnectedAccountId,
+      );
+    });
+  });
+});