fix: don't move a queued message to drafts when an earlier attempt already sent it

locales/rn/en.json

@@ -1004,6 +1004,7 @@
     "section_personal": "My templates"
   },
   "outbox": {
+    "already_sent": "This message was already sent",
     "discard": "Discard",
     "discard_body": "It will not be sent, and its text is lost.",
     "discard_title": "Discard this message?",

src/api/__tests__/queued-send.test.ts

@@ -18,7 +18,7 @@ vi.mock('../../lib/compose-send-account', () => ({
 }));
 
 import { jmapClient } from '../jmap-client';
-import { findCopiesByMessageId, runQueuedSend } from '../queued-send';
+import { findCopiesByMessageId, runQueuedSend, wasQueuedSendSent } from '../queued-send';
 import { newQueuedSend, type QueuedSendPayload } from '../../lib/send-queue';
 
 const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
@@ -85,6 +85,31 @@ describe('runQueuedSend', () => {
   });
 });
 
+describe('wasQueuedSendSent', () => {
+  it('finds a copy an earlier attempt sent, and removes the old draft', async () => {
+    mockRequest.mockResolvedValueOnce(found([
+      { id: 'draft-1', keywords: { $draft: true }, messageId: ['m1@x.de'] },
+      { id: 'sent-1', keywords: { $seen: true }, messageId: ['m1@x.de'] },
+    ]));
+    expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, true), [])).toBe(true);
+    expect(destroyEmails).toHaveBeenCalledWith(['draft-1'], 'acc-1');
+  });
+
+  it('says not sent when only drafts are there, and removes nothing', async () => {
+    mockRequest.mockResolvedValueOnce(found([
+      { id: 'draft-1', keywords: { $draft: true }, messageId: ['m1@x.de'] },
+      { id: 'left-1', keywords: { $draft: true, $seen: true }, messageId: ['m1@x.de'] },
+    ]));
+    expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, true), [])).toBe(false);
+    expect(destroyEmails).not.toHaveBeenCalled();
+  });
+
+  it('does not look for a message no attempt reached the server with', async () => {
+    expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, false), [])).toBe(false);
+    expect(mockRequest).not.toHaveBeenCalled();
+  });
+});
+
 describe('findCopiesByMessageId', () => {
   it('reads the recent messages of Sent and Drafts and keeps exact matches only', async () => {
     mockRequest.mockResolvedValueOnce(found([

src/api/queued-send.ts

@@ -8,7 +8,7 @@ import type { Mailbox } from './types';
 import { sendTargetFor } from '../lib/compose-send-account';
 import { supportsSubmissionExtension } from '../lib/submission-options';
 import { stripMessageIdBrackets } from '../lib/email-threading';
-import { holdForScheduled, verdictFromCopies, type FoundCopy, type QueuedSend } from '../lib/send-queue';
+import { holdForScheduled, verdictFromCopies, type CopyVerdict, type FoundCopy, type QueuedSend } from '../lib/send-queue';
 
 /** How far before the queueing a copy can be dated (the server's clock may differ). */
 const CLOCK_SKEW_MS = 10 * 60_000;
@@ -68,34 +68,64 @@ export type QueuedSendOutcome =
  * went out; copies a refused attempt left in Drafts are removed before the
  * new attempt.
  */
-export async function runQueuedSend(send: QueuedSend, mailboxes: Mailbox[], now = Date.now()): Promise<QueuedSendOutcome> {
+/** The account and folders a queued message goes through, as the composer works them out. */
+async function queuedSendTarget(send: QueuedSend, mailboxes: Mailbox[]) {
   const p = send.payload;
   const target = await sendTargetFor(p.identity, p.fromEmail, mailboxes);
-  const accountId = target.accountId ?? jmapClient.accountId;
-  const draftAccountId = p.draftAccountId ?? jmapClient.accountId;
+  return {
+    target,
+    accountId: target.accountId ?? jmapClient.accountId,
+    draftAccountId: p.draftAccountId ?? jmapClient.accountId,
+    sentMailboxId: target.sentMailboxId ?? p.sentMailboxId,
+    draftsMailboxId: target.accountId ? target.draftsMailboxId : p.draftsMailboxId,
+  };
+}
 
-  const sentMailboxId = target.sentMailboxId ?? p.sentMailboxId;
-  const draftsMailboxId = target.accountId ? target.draftsMailboxId : p.draftsMailboxId;
+type Resolved = Awaited<ReturnType<typeof queuedSendTarget>>;
+
+/**
+ * Whether an earlier, unanswered attempt at `send` went out. When it did, the
+ * old draft that send would have removed is removed now.
+ */
+async function earlierAttempt(send: QueuedSend, r: Resolved): Promise<CopyVerdict> {
+  const p = send.payload;
+  const copies = await findCopiesByMessageId(p.outgoing.messageId!, {
+    accountId: r.target.accountId,
+    mailboxIds: [r.sentMailboxId, r.draftsMailboxId ?? ''],
+    since: send.createdAt,
+  });
+  // The autosaved draft lives in this account only when the draft did.
+  const draftHere = p.draftId && r.draftAccountId === r.accountId ? p.draftId : undefined;
+  const verdict = verdictFromCopies(copies, draftHere);
+  if (verdict.kind === 'sent' && p.draftId && p.draftId !== verdict.copyId) {
+    // The send went out but its answer was lost, so the old draft it would
+    // have removed is still there.
+    await destroyEmails([p.draftId], r.draftAccountId).catch((err) => {
+      console.warn('[send-queue] could not remove the draft of a sent message', err);
+    });
+  }
+  return verdict;
+}
+
+/**
+ * Before a queued message is taken back (moved to Drafts): true when an
+ * earlier attempt that may have reached the server did send it. A message no
+ * attempt reached the server with is never looked up.
+ */
+export async function wasQueuedSendSent(send: QueuedSend, mailboxes: Mailbox[]): Promise<boolean> {
+  if (!send.maybeSent || !send.payload.outgoing.messageId) return false;
+  const verdict = await earlierAttempt(send, await queuedSendTarget(send, mailboxes));
+  return verdict.kind === 'sent';
+}
+
+export async function runQueuedSend(send: QueuedSend, mailboxes: Mailbox[], now = Date.now()): Promise<QueuedSendOutcome> {
+  const p = send.payload;
+  const resolved = await queuedSendTarget(send, mailboxes);
+  const { target, accountId, sentMailboxId, draftsMailboxId } = resolved;
 
   if (send.maybeSent && p.outgoing.messageId) {
-    const copies = await findCopiesByMessageId(p.outgoing.messageId, {
-      accountId: target.accountId,
-      mailboxIds: [sentMailboxId, draftsMailboxId ?? ''],
-      since: send.createdAt,
-    });
-    // The autosaved draft lives in this account only when the draft did.
-    const draftHere = p.draftId && draftAccountId === accountId ? p.draftId : undefined;
-    const verdict = verdictFromCopies(copies, draftHere);
-    if (verdict.kind === 'sent') {
-      // The send went out but its answer was lost, so the old draft it would
-      // have removed is still there.
-      if (p.draftId && p.draftId !== verdict.copyId) {
-        await destroyEmails([p.draftId], draftAccountId).catch((err) => {
-          console.warn('[send-queue] could not remove the draft of a sent message', err);
-        });
-      }
-      return { kind: 'alreadySent', accountId: target.accountId };
-    }
+    const verdict = await earlierAttempt(send, resolved);
+    if (verdict.kind === 'sent') return { kind: 'alreadySent', accountId: target.accountId };
     if (verdict.staleIds.length > 0) {
       await destroyEmails(verdict.staleIds, accountId);
     }

src/stores/__tests__/outbox-send-queue.test.ts

@@ -25,8 +25,10 @@ vi.mock('../../api/email', () => ({
 }));
 
 const runQueuedSend = vi.fn(async (..._a: any[]): Promise<any> => ({ kind: 'sent', result: { scheduled: false } }));
+const wasQueuedSendSent = vi.fn(async (..._a: any[]) => false);
 vi.mock('../../api/queued-send', () => ({
   runQueuedSend: (...a: any[]) => runQueuedSend(...a),
+  wasQueuedSendSent: (...a: any[]) => wasQueuedSendSent(...a),
 }));
 vi.mock('../email-store', () => ({
   useEmailStore: { getState: () => ({ mailboxes: [{ id: 'sent', role: 'sent' }] }) },
@@ -221,6 +223,38 @@ describe('offline send queue', () => {
     expect(useOutboxStore.getState().sends[0]).toMatchObject({ id: again, status: 'failed' });
   });
 
+  it('does not turn a message an earlier attempt sent into a draft, and says it was sent', async () => {
+    const id = useOutboxStore.getState().enqueueSend(payload('went-out'), { maybeSent: true })!;
+    wasQueuedSendSent.mockResolvedValueOnce(true);
+    expect(await useOutboxStore.getState().cancelSend(id, { keepAsDraft: true })).toBe(true);
+    expect(wasQueuedSendSent).toHaveBeenCalledWith(expect.objectContaining({ id, maybeSent: true }), [{ id: 'sent', role: 'sent' }]);
+    expect(saveDraftVersion).not.toHaveBeenCalled();
+    expect(useOutboxStore.getState().sends).toEqual([]);
+    await flushed();
+    expect(toast.info).toHaveBeenCalledWith('This message was already sent', { message: 'went-out' });
+    expect(patchKeywordsForEmails).toHaveBeenCalledWith(['orig-1'], { $answered: true }, undefined);
+  });
+
+  it('moves a marked message to Drafts when no earlier attempt sent it, and looks only for marked ones', async () => {
+    const marked = useOutboxStore.getState().enqueueSend(payload('marked'), { maybeSent: true })!;
+    expect(await useOutboxStore.getState().cancelSend(marked, { keepAsDraft: true })).toBe(true);
+    expect(wasQueuedSendSent).toHaveBeenCalledTimes(1);
+    expect(saveDraftVersion).toHaveBeenCalledTimes(1);
+
+    const plain = useOutboxStore.getState().enqueueSend(payload('plain'))!;
+    expect(await useOutboxStore.getState().cancelSend(plain, { keepAsDraft: true })).toBe(true);
+    expect(wasQueuedSendSent).toHaveBeenCalledTimes(1);
+    expect(saveDraftVersion).toHaveBeenCalledTimes(2);
+  });
+
+  it('keeps a marked message in the Outbox when the lookup fails', async () => {
+    const id = useOutboxStore.getState().enqueueSend(payload('unknown'), { maybeSent: true })!;
+    wasQueuedSendSent.mockRejectedValueOnce(named('NetworkError', 'Network request failed'));
+    expect(await useOutboxStore.getState().cancelSend(id, { keepAsDraft: true })).toBe(false);
+    expect(saveDraftVersion).not.toHaveBeenCalled();
+    expect(useOutboxStore.getState().sends[0]).toMatchObject({ id, status: 'failed' });
+  });
+
   it('drops the queued messages with the rest when the server behind the account changed', async () => {
     useOutboxStore.getState().enqueueSend(payload());
     expect(await useOutboxStore.getState().clearAccount(ACCOUNT)).toBe(1);

src/stores/outbox-store.ts

@@ -562,6 +562,22 @@ export const useOutboxStore = create<OutboxState>((set, get) => ({
     if (!opts?.keepAsDraft) return true;
     const p = send.payload;
     try {
+      // An attempt that may have reached the server is looked for first, as
+      // before a resend: a message that went out must not come back as a
+      // draft.
+      if (send.maybeSent) {
+        const [{ wasQueuedSendSent }, { useEmailStore }] = await Promise.all([
+          import('../api/queued-send'),
+          import('./email-store'),
+        ]);
+        if (await wasQueuedSendSent(send, useEmailStore.getState().mailboxes)) {
+          if (p.original) {
+            patchKeywordsForEmails([p.original.emailId], { [p.original.keyword]: true }, p.original.accountId).catch(() => undefined);
+          }
+          void notifyAlreadySent(send);
+          return true;
+        }
+      }
       if (!p.draftsMailboxId) throw new Error('No Drafts folder');
       const { saveDraftVersion } = await import('../api/email');
       await saveDraftVersion(p.outgoing, p.draftsMailboxId, p.draftId, undefined, p.draftAccountId);
@@ -732,6 +748,19 @@ async function afterQueuedSend(send: QueuedSend, outcome: QueuedSendOutcomeLike)
   }
 }
 
+/** Taking back a message found to have gone out already: nothing became a draft. */
+async function notifyAlreadySent(send: QueuedSend): Promise<void> {
+  try {
+    const [{ toast }, { useLocaleStore }] = await Promise.all([import('./toast-store'), import('./locale-store')]);
+    const t = useLocaleStore.getState().t;
+    toast.info(t('outbox.already_sent', 'This message was already sent'), {
+      message: send.payload.outgoing.subject || undefined,
+    });
+  } catch (err) {
+    console.warn('[outbox] could not report an already sent message', err);
+  }
+}
+
 async function notifyQueuedSendFailed(send: QueuedSend, message: string): Promise<void> {
   try {
     const [{ toast }, { useLocaleStore }] = await Promise.all([import('./toast-store'), import('./locale-store')]);