fix: keep a queued message without a drafts folder from passing as sent when its submission failed

src/api/__tests__/email-submission-refusals.test.ts

@@ -40,6 +40,7 @@ import {
   listScheduledEmails, RescheduleLostError, RescheduleRestoredError, rescheduleScheduledSend, sendEmail,
 } from '../email';
 import { RecipientsRefusedError, jmapErrorType } from '../jmap-result';
+import { verdictFromCopies } from '../../lib/send-queue';
 import { pruneEmailSubmissions, resetSubmissionPrune } from '../submissions';
 import {
   createFakeSubmissionServer, RATE_LIMITED, RELAY_DENIED, type StalwartVersion,
@@ -87,6 +88,33 @@ afterEach(() => {
   expect(fake.server?.violations ?? []).toEqual([]);
 });
 
+describe('a send from an account without a Drafts folder', () => {
+  // The copy is made in Sent right away. If the submission is refused and
+  // the answer is lost, that copy must not look sent to the offline queue,
+  // which takes a copy without $draft as proof the message went out.
+  it('leaves a copy a refused, unanswered submission made marked as a draft', async () => {
+    const server = setup('1.0', { forbiddenAccounts: ['shared-1'] });
+    server.afterEachRequest(() => { throw new TypeError('Network request failed'); });
+
+    await expect(sendEmail({ ...OUTGOING, messageId: 'm1@example.com' }, 'id-1', 'sent', undefined, { accountId: 'shared-1' }))
+      .rejects.toThrow('Network request failed');
+
+    const copies = [...server.emails.values()];
+    expect(copies).toEqual([expect.objectContaining({ mailboxIds: { sent: true }, keywords: { $seen: true, $draft: true } })]);
+    expect(verdictFromCopies(copies)).toEqual({ kind: 'resend', staleIds: [copies[0].id] });
+  });
+
+  it('takes $draft off the copy once the submission went through', async () => {
+    const server = setup('1.0');
+
+    await sendEmail(OUTGOING, 'id-1', 'sent', undefined, {});
+
+    const copies = [...server.emails.values()];
+    expect(copies).toEqual([expect.objectContaining({ mailboxIds: { sent: true }, keywords: { $seen: true } })]);
+    expect(verdictFromCopies(copies)).toEqual({ kind: 'sent', copyId: copies[0].id });
+  });
+});
+
 describe('a send whose recipients the server refuses', () => {
   it.each(['1.0', '0.16.23', '0.16.10', '0.16.6'] as StalwartVersion[])(
     'Stalwart %s: fails when every recipient was refused, and keeps the previous draft',

src/api/__tests__/live-stalwart-send-queue.test.ts

@@ -101,6 +101,18 @@ live('live Stalwart: offline send queue', () => {
     expect(copies.map((c) => c.id)).toEqual([first.emailId]);
   }, 60_000);
 
+  it('takes $draft off a copy sent without a Drafts folder only once it went out', async () => {
+    const outgoing = message('no-drafts');
+    // Created in Sent with $draft; onSuccessUpdateEmail takes it off.
+    const result = await sendEmail(outgoing, identity.id, sent, undefined, {});
+    const copies = await findCopiesByMessageId(outgoing.messageId, lookup());
+    expect(copies.map((c) => c.id)).toEqual([result.emailId]);
+    expect(copies[0].keywords?.$draft).toBeFalsy();
+    expect(copies[0].keywords?.$seen).toBe(true);
+    const outcome = await runQueuedSend(newQueuedSend('q3', { ...payloadFor(outgoing), draftsMailboxId: undefined }, Date.now(), true), boxes);
+    expect(outcome.kind).toBe('alreadySent');
+  }, 60_000);
+
   it('sends a marked message whose only copy is its draft, and removes the draft', async () => {
     const outgoing = message('draft-only');
     const draft = await saveDraftVersion(outgoing, drafts);

src/api/email.ts

@@ -1781,8 +1781,12 @@ export async function sendEmail(
     emailCreate.mailboxIds = { [opts!.draftsMailboxId!]: true };
     emailCreate.keywords = { $seen: true, $draft: true };
   } else {
+    // No Drafts folder: created in Sent, but still with `$draft` until the
+    // submission succeeds, so a copy whose submission was refused (with the
+    // answer lost) never passes for a sent message. The send queue takes a
+    // copy without `$draft` as proof the message went out (verdictFromCopies).
     emailCreate.mailboxIds = { [sentMailboxId]: true };
-    emailCreate.keywords = { $seen: true };
+    emailCreate.keywords = { $seen: true, $draft: true };
   }
   const createCall: [string, Record<string, unknown>, string] = raw
     ? ['Email/import', { accountId, emails: { draft: emailCreate } }, '0']
@@ -1818,14 +1822,11 @@ export async function sendEmail(
     accountId,
     create: { 'sub-1': submissionCreate },
   };
-  if (viaDrafts) {
+  submissionArgs.onSuccessUpdateEmail = {
-    submissionArgs.onSuccessUpdateEmail = {
+    '#sub-1': viaDrafts
-      '#sub-1': {
+      ? { mailboxIds: { [sentMailboxId]: true }, [keywordPointer('$draft')]: null }
-        mailboxIds: { [sentMailboxId]: true },
+      : { [keywordPointer('$draft')]: null },
-        [keywordPointer('$draft')]: null,
+  };
-      },
-    };
-  }
 
   let res = await jmapClient.request(
     [

src/lib/send-queue.ts

@@ -130,9 +130,10 @@ export type CopyVerdict =
 
 /**
  * What the copies with the message's Message-ID say. A send creates the
- * message as a draft and the submission's onSuccessUpdateEmail files it in
+ * message as a draft (in Drafts, or in Sent when the account has no Drafts
- * Sent without `$draft` in the same request, so a copy without `$draft` went
+ * folder) and only the submission's onSuccessUpdateEmail takes `$draft` off
- * out (or is held for a scheduled time). Copies still marked `$draft` are
+ * (and files it in Sent) in the same request, so a copy without `$draft`
+ * went out (or is held for a scheduled time). Copies still marked `$draft` are
  * the autosaved draft (`draftId`, kept until the send succeeds) and copies
  * a refused submission left behind with its answer lost: those go before
  * the message is sent again.