cold email: fail toward not blocking, and stop clobbering learned patterns

apps/web/utils/ai/choose-rule/execute.test.ts

@@ -220,6 +220,33 @@ describe("executeAct", () => {
     });
   });
 
+  it("keeps the rule APPLIED when an action skips itself on purpose", async () => {
+    mockRunActionFunction.mockResolvedValueOnce({ skipped: true });
+
+    const executedRule = {
+      ...baseExecutedRule,
+      actionItems: [{ id: "action-1", type: ActionType.NOTIFY_SENDER }],
+    } as any;
+
+    const result = await executeAct({
+      client: mockClient,
+      executedRule,
+      message,
+      emailAccount,
+      logger,
+    });
+
+    expect(result).toBe(ExecutedRuleStatus.APPLIED);
+    expect(mockExecutedActionUpdate).toHaveBeenCalledWith({
+      where: { id: "action-1" },
+      data: {
+        executionStatus: "SKIPPED",
+        executedAt: expect.any(Date),
+        executionError: Prisma.DbNull,
+      },
+    });
+  });
+
   it("continues later messaging notifications after one delivery failure", async () => {
     mockRunActionFunction
       .mockResolvedValueOnce({

apps/web/utils/cold-email/has-prior-contact.test.ts

@@ -0,0 +1,69 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { hasPriorContactOrAssumeYes } from "./has-prior-contact";
+import { createTestLogger } from "@/__tests__/helpers";
+
+const logger = createTestLogger();
+
+describe("hasPriorContactOrAssumeYes", () => {
+  const provider = {
+    hasPreviousCommunicationsWithSenderOrDomain: vi.fn(),
+  };
+
+  const check = (overrides: Record<string, unknown> = {}) =>
+    hasPriorContactOrAssumeYes({
+      provider: provider as never,
+      from: "sender@example.com",
+      date: new Date(),
+      messageId: "msg-1",
+      logger,
+      ...overrides,
+    });
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("reports what the provider found", async () => {
+    provider.hasPreviousCommunicationsWithSenderOrDomain.mockResolvedValue(
+      false,
+    );
+
+    await expect(check()).resolves.toBe(false);
+  });
+
+  // Each of these would otherwise read as "no prior contact", which is the input that
+  // pushes the cold email blocker toward blocking a sender we could not verify.
+  it.each([
+    [
+      "the provider errors",
+      {},
+      () =>
+        provider.hasPreviousCommunicationsWithSenderOrDomain.mockRejectedValue(
+          new Error("api down"),
+        ),
+    ],
+    ["the sender is blank", { from: " " }, () => {}],
+    ["the date is missing", { date: undefined }, () => {}],
+    ["the date is invalid", { date: new Date(Number.NaN) }, () => {}],
+    ["the message id is missing", { messageId: undefined }, () => {}],
+  ])("assumes contact when %s", async (_name, overrides, arrange) => {
+    provider.hasPreviousCommunicationsWithSenderOrDomain.mockResolvedValue(
+      false,
+    );
+    arrange();
+
+    await expect(check(overrides)).resolves.toBe(true);
+  });
+
+  it.each([
+    { from: " " },
+    { date: undefined },
+    { messageId: undefined },
+  ])("does not call the provider when contact cannot be identified", async (overrides) => {
+    await check(overrides);
+
+    expect(
+      provider.hasPreviousCommunicationsWithSenderOrDomain,
+    ).not.toHaveBeenCalled();
+  });
+});

apps/web/utils/cold-email/has-prior-contact.ts

@@ -0,0 +1,53 @@
+import type { EmailProvider } from "@/utils/email/types";
+import type { Logger } from "@/utils/logger";
+
+/**
+ * Whether the user has corresponded with this sender before, assuming yes whenever we
+ * cannot tell.
+ *
+ * Blocking a sender is compounding: it labels, archives, and on many accounts emails
+ * them to say their message was unsolicited. Letting a cold email through costs one
+ * email in the inbox. So a missing date, an unreadable message, or a provider outage
+ * must all resolve toward leaving the sender alone rather than toward blocking them.
+ */
+export async function hasPriorContactOrAssumeYes({
+  provider,
+  from,
+  date,
+  messageId,
+  logger,
+}: {
+  provider: EmailProvider;
+  from: string;
+  date: Date | undefined;
+  messageId: string | undefined;
+  logger: Logger;
+}): Promise<boolean> {
+  if (
+    !from.trim() ||
+    !date ||
+    Number.isNaN(date.getTime()) ||
+    !messageId?.trim()
+  ) {
+    logger.warn(
+      "Assuming prior contact - message is missing a sender, date, or id",
+    );
+    return true;
+  }
+
+  try {
+    return await provider.hasPreviousCommunicationsWithSenderOrDomain({
+      from,
+      date,
+      messageId,
+    });
+  } catch (error) {
+    logger.warn(
+      "Assuming prior contact - could not check for previous emails",
+      {
+        error,
+      },
+    );
+    return true;
+  }
+}

apps/web/utils/cold-email/is-cold-email.test.ts

@@ -141,6 +141,85 @@ describe("isColdEmail", () => {
     });
   });
 
+  // Guarded here rather than only at the actions, so a colleague is never labelled
+  // or archived either.
+  it("should not classify a colleague as cold", async () => {
+    vi.mocked(prisma.groupItem.findFirst).mockResolvedValue(null);
+
+    const result = await isColdEmail({
+      email: {
+        id: "msg-internal",
+        from: "ceo@company.com",
+        to: "user@company.com",
+        subject: "Quick favour",
+        content: "Can you take a look at this?",
+        date: new Date(),
+      },
+      emailAccount: getEmailAccount({
+        id: "test-account-id",
+        email: "user@company.com",
+      }),
+      provider: mockProvider as never,
+      coldEmailRule: { instructions: "test instructions", groupId: "group-id" },
+    });
+
+    expect(result.isColdEmail).toBe(false);
+    expect(
+      mockProvider.hasPreviousCommunicationsWithSenderOrDomain,
+    ).not.toHaveBeenCalled();
+  });
+
+  it("should not classify a colleague as cold when a learned pattern exists", async () => {
+    vi.mocked(prisma.groupItem.findFirst).mockResolvedValue({
+      id: "group-item-id",
+      type: GroupItemType.FROM,
+      value: "ceo@company.com",
+      exclude: false,
+      group: { id: "group-id", name: "Cold Email" },
+    } as any);
+
+    const result = await isColdEmail({
+      email: {
+        id: "msg-internal",
+        from: "ceo@company.com",
+        to: "user@company.com",
+        subject: "Quick favour",
+        content: "Can you take a look at this?",
+        date: new Date(),
+      },
+      emailAccount: getEmailAccount({
+        id: "test-account-id",
+        email: "user@company.com",
+      }),
+      provider: mockProvider as never,
+      coldEmailRule: { instructions: "test instructions", groupId: "group-id" },
+    });
+
+    expect(result.isColdEmail).toBe(false);
+  });
+
+  // Blocking a sender we could not verify is worse than missing a cold email.
+  it("should not classify as cold when prior contact cannot be checked", async () => {
+    vi.mocked(prisma.groupItem.findFirst).mockResolvedValue(null);
+
+    const result = await isColdEmail({
+      email: {
+        id: "msg-no-date",
+        from: "unknown@example.com",
+        to: "user@test.com",
+        subject: "Hello",
+        content: "Hello",
+        date: undefined as never,
+      },
+      emailAccount: getEmailAccount({ id: "test-account-id" }),
+      provider: mockProvider as never,
+      coldEmailRule: { instructions: "test instructions", groupId: "group-id" },
+    });
+
+    expect(result.isColdEmail).toBe(false);
+    expect(result.reason).toBe("hasPreviousEmail");
+  });
+
   it("should handle various email formats consistently", async () => {
     const emailAccount = getEmailAccount({ id: "test-account-id" });
     const normalizedEmail = "sender@example.com";

apps/web/utils/cold-email/is-cold-email.ts

@@ -10,7 +10,8 @@ import type { EmailForLLM } from "@/utils/types";
 import type { EmailProvider } from "@/utils/email/types";
 import { getModel, type ModelType } from "@/utils/llms/model";
 import { createGenerateObject } from "@/utils/llms";
-import { extractEmailAddress } from "@/utils/email";
+import { extractEmailAddress, isSameOrganization } from "@/utils/email";
+import { hasPriorContactOrAssumeYes } from "@/utils/cold-email/has-prior-contact";
 
 export const COLD_EMAIL_FOLDER_NAME = "Cold Emails";
 
@@ -52,6 +53,13 @@ export async function isColdEmail({
 
   logger.info("Checking is cold email");
 
+  // Nobody at your own company is a cold emailer. Checked here rather than only at the
+  // actions, so a colleague is never labelled or archived either.
+  if (isSameOrganization(email.from, emailAccount.email)) {
+    logger.info("Sender is internal");
+    return { isColdEmail: false, reason: "hasPreviousEmail" };
+  }
+
   // Check if we marked it as a cold email already
   const groupId = coldEmailRule?.groupId;
   let patternMatch:
@@ -95,14 +103,13 @@ export async function isColdEmail({
     return { isColdEmail: false, reason: "excluded" };
   }
 
-  const hasPreviousEmail =
-    email.date && email.id
-      ? await provider.hasPreviousCommunicationsWithSenderOrDomain({
-          from: extractEmailAddress(email.from) || email.from,
-          date: email.date,
-          messageId: email.id,
-        })
-      : false;
+  const hasPreviousEmail = await hasPriorContactOrAssumeYes({
+    provider,
+    from: extractEmailAddress(email.from) || email.from,
+    date: email.date,
+    messageId: email.id,
+    logger,
+  });
 
   if (hasPreviousEmail) {
     logger.info("Has previous email");