apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts (1)

595-597: Minor: await mailbox waits in parallel to speed up test

-    await userA.mailbox.waitForMessagesWithSubject(subject);
-    await userB.mailbox.waitForMessagesWithSubject(subject);
-    await userC.mailbox.waitForMessagesWithSubject(subject);
+    await Promise.all([
+      userA.mailbox.waitForMessagesWithSubject(subject),
+      userB.mailbox.waitForMessagesWithSubject(subject),
+      userC.mailbox.waitForMessagesWithSubject(subject),
+    ]);

apps/e2e/tests/helpers.ts (2)

200-203: Consider options on waitForMessagesWithSubject (includeBody, retries, interval)

Proposed signature change (backward compatible defaults keep current behavior):

-  public readonly waitForMessagesWithSubject: (subject: string) => Promise<MailboxMessage[]>;
+  public readonly waitForMessagesWithSubject: (
+    subject: string,
+    options?: { includeBody?: boolean; maxRetries?: number; intervalMs?: number }
+  ) => Promise<MailboxMessage[]>;

---

227-238: Optional: allow returning messages with bodies (unblocks body assertions without a second fetch)

This keeps default noBody behavior for existing snapshots, but enables consumers to request bodies when needed.

-    this.waitForMessagesWithSubject = async (subject: string) => {
-      const maxRetries = 20;
-      for (let i = 0; i < maxRetries; i++) {
-        const messages = await this.fetchMessages({ noBody: true });
-        const withSubject = messages.filter(m => m.subject === subject);
-        if (withSubject.length > 0) {
-          return withSubject;
-        }
-        await wait(200);
-      }
-      throw new Error(`Message with subject ${subject} not found`);
-    };
+    this.waitForMessagesWithSubject = async (
+      subject: string,
+      options?: { includeBody?: boolean; maxRetries?: number; intervalMs?: number }
+    ) => {
+      const maxRetries = options?.maxRetries ?? 20;
+      const intervalMs = options?.intervalMs ?? 200;
+      for (let i = 0; i < maxRetries; i++) {
+        const messages = await this.fetchMessages({ noBody: true });
+        const withSubject = messages.filter(m => m.subject === subject);
+        if (withSubject.length > 0) {
+          if (options?.includeBody) {
+            const full = await this.fetchMessages(); // uses cache
+            return full.filter(m => m.subject === subject);
+          }
+          return withSubject;
+        }
+        await wait(intervalMs);
+      }
+      throw new Error(`Message with subject ${subject} not found after ${maxRetries} retries`);
+    };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

apps/e2e/tests/helpers.ts (1)

235-235: Re: automated comment about runAsynchronously — this is already awaited.
The pattern in Line 235 uses await wait(200), not fire-and-forget; the bot warning looks like a false positive.

apps/e2e/tests/helpers.ts (2)

210-226: Avoid N+1 full-message fetches when noBody is true.
When callers only need headers/subject (e.g., polling), short-circuit using the list payload to skip per-message detail fetches. Keeps Map usage (preferred over Record) and reduces network/CPU.

Apply this diff:

 this.fetchMessages = async ({ noBody } = {}) => {
-  const res = await niceFetch(new URL(`/api/v1/mailbox/${encodeURIComponent(mailboxName)}`, INBUCKET_API_URL));
-  return await Promise.all((res.body as any[]).map(async (message) => {
-    let fullMessage: any;
-    if (fullMessageCache.has(message.id)) {
-      fullMessage = fullMessageCache.get(message.id);
-    } else {
-      const fullMessageRes = await niceFetch(new URL(`/api/v1/mailbox/${encodeURIComponent(mailboxName)}/${message.id}`, INBUCKET_API_URL));
-      fullMessage = fullMessageRes.body;
-      fullMessageCache.set(message.id, fullMessage);
-    }
-    const messagePart = noBody ? omit(fullMessage, ["body", "attachments"]) : fullMessage;
-    return new MailboxMessage(messagePart);
-  }));
+  const res = await niceFetch(new URL(`/api/v1/mailbox/${encodeURIComponent(mailboxName)}`, INBUCKET_API_URL));
+  const list = res.body as any[];
+  if (noBody) {
+    // Build from list payload to avoid N extra network calls
+    return list.map((message) => new MailboxMessage(omit(message, ["body", "attachments"])));
+  }
+  return await Promise.all(list.map(async (message) => {
+    let fullMessage: any = fullMessageCache.get(message.id);
+    if (!fullMessage) {
+      const fullMessageRes = await niceFetch(new URL(`/api/v1/mailbox/${encodeURIComponent(mailboxName)}/${message.id}`, INBUCKET_API_URL));
+      fullMessage = fullMessageRes.body;
+      fullMessageCache.set(message.id, fullMessage);
+    }
+    return new MailboxMessage(fullMessage);
+  }));
 };

If the list payload lacks some fields needed by MailboxMessage in your snapshots, keep the current path for those cases and only short-circuit when safe.

---

227-238: Make polling cheaper and error messages more actionable.
Use noBody during polling and include mailbox/elapsed time + last subjects to ease debugging timeouts.

 this.waitForMessagesWithSubject = async (subject: string) => {
-  const maxRetries = 20;
-  for (let i = 0; i < maxRetries; i++) {
-    const messages = await this.fetchMessages();
-    const withSubject = messages.filter(m => m.subject === subject);
+  const maxRetries = 20;
+  const intervalMs = 200;
+  let lastSubjects: string[] = [];
+  for (let i = 0; i < maxRetries; i++) {
+    const messages = await this.fetchMessages({ noBody: true });
+    lastSubjects = messages.map(m => m.subject);
+    const withSubject = messages.filter(m => m.subject === subject);
     if (withSubject.length > 0) {
       return withSubject;
     }
-    await wait(200);
+    await wait(intervalMs);
   }
-  throw new Error(`Message with subject ${subject} not found`);
+  throw new Error(`Message with subject "${subject}" not found for ${this.emailAddress} after ${maxRetries} retries (~${maxRetries * intervalMs}ms). Last subjects seen: ${lastSubjects.slice(0, 10).join(", ")}`);
 };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts (5)

297-312: Assert cardinality and consider snapshotting only stable fields to reduce flakiness

Add an explicit length assertion so the test fails clearly if multiple emails with the same subject arrive. Optionally, snapshot a stable projection instead of the full message (body can churn).

Minimal addition:

   const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
+  expect(messages).toHaveLength(1);
   expect(messages).toMatchInlineSnapshot(`

Optional alternative (snapshot stable fields):

-  const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
-  expect(messages).toMatchInlineSnapshot(`
-    [
-      MailboxMessage {
-        "attachments": [],
-        "body": {
-          "html": "http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E",
-          "text": "http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E",
-        },
-        "from": "Test Project <test@example.com>",
-        "subject": "Custom Test Email Subject",
-        "to": ["<unindexed-mailbox--<stripped UUID>@stack-generated.example.com>"],
-        <some fields may have been hidden>,
-      },
-    ]
-  `);
+  const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
+  expect(messages).toHaveLength(1);
+  expect(messages.map(m => ({
+    from: m.from,
+    to: m.to,
+    subject: m.subject,
+    attachments: m.attachments.length,
+  }))).toMatchInlineSnapshot(`
+    [
+      {
+        "from": "Test Project <test@example.com>",
+        "to": ["<unindexed-mailbox--<stripped UUID>@stack-generated.example.com>"],
+        "subject": "Custom Test Email Subject",
+        "attachments": 0,
+      },
+    ]
+  `);

---

417-417: Validate the awaited result, not just completion

Capture the returned array and assert at least one (ideally exactly one) message to tighten the check.

-  await user.mailbox.waitForMessagesWithSubject("Overridden Subject");
+  const msgs = await user.mailbox.waitForMessagesWithSubject("Overridden Subject");
+  expect(msgs).toHaveLength(1);

---

600-602: Parallelize mailbox waits to speed up the test

Waiting sequentially adds unnecessary latency; use Promise.all.

-    await userA.mailbox.waitForMessagesWithSubject(subject);
-    await userB.mailbox.waitForMessagesWithSubject(subject);
-    await userC.mailbox.waitForMessagesWithSubject(subject);
+    await Promise.all(
+      [userA, userB, userC].map(u => u.mailbox.waitForMessagesWithSubject(subject))
+    );

---

746-746: Assert cardinality for the transactional email

Ensure exactly one message arrived with the subject.

-    await user.mailbox.waitForMessagesWithSubject("Transactional Test Subject");
+    const msgs = await user.mailbox.waitForMessagesWithSubject("Transactional Test Subject");
+    expect(msgs).toHaveLength(1);

---

785-785: Assert cardinality for the default category email

Same rationale as above: make the expectation explicit.

-    await user.mailbox.waitForMessagesWithSubject("Default Category Test Subject");
+    const msgs = await user.mailbox.waitForMessagesWithSubject("Default Category Test Subject");
+    expect(msgs).toHaveLength(1);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro