fix: keep a rescheduled send from going out twice or not at all when the server's answer is lost

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

@@ -218,6 +218,96 @@ describe('rescheduleScheduledSend', () => {
     vi.mocked(Date.now).mockRestore();
   });
 
+  describe('answers that never came', () => {
+    const status = (undoStatus: string) => ({
+      methodResponses: [['EmailSubmission/get', { list: [{ id: 'sub-1', undoStatus }] }, '0']],
+    });
+    const records = (list: Array<Record<string, unknown>>) => ({
+      methodResponses: [
+        ['EmailSubmission/query', { ids: list.map((s) => s.id) }, '0'],
+        ['EmailSubmission/get', { list }, '1'],
+      ],
+    });
+    const creates = () => setCalls().filter(([created]) => created.length > 0).length;
+
+    it('goes on to the replacement when the cancel went through but its answer was lost', async () => {
+      mockRequest
+        .mockResolvedValueOnce(LOOKUP)
+        .mockRejectedValueOnce(new Error('Request timed out'))
+        .mockResolvedValueOnce(status('canceled'))
+        .mockResolvedValueOnce(DESTROYED('sub-1'))
+        .mockResolvedValueOnce({ methodResponses: REPLACED });
+
+      const result = await rescheduleScheduledSend(SCHEDULED, 0);
+
+      expect(result.emailSubmissionId).toBe('sub-2');
+      // The canceled record goes, as after a confirmed cancel.
+      expect(setCalls()).toEqual([[[], ['sub-1'], []], [[], [], ['sub-1']], [['replacement'], [], []]]);
+    });
+
+    it('still makes no replacement when the server refused the cancel and the original was cancelled elsewhere', async () => {
+      mockRequest
+        .mockResolvedValueOnce(LOOKUP)
+        .mockResolvedValueOnce(set({ notUpdated: { 'sub-1': { type: 'invalidProperties' } } }))
+        .mockResolvedValueOnce(status('canceled'));
+
+      const err = await rescheduleScheduledSend(SCHEDULED, 0).catch((e: unknown) => e);
+
+      expect(err).toBeInstanceOf(NoLongerScheduledError);
+      expect(creates()).toBe(0);
+    });
+
+    it('takes a replacement the server made for the answer that was lost, and does not put the original back', async () => {
+      mockRequest
+        .mockResolvedValueOnce(LOOKUP)
+        .mockResolvedValueOnce(CANCELLED)
+        .mockResolvedValueOnce(DESTROYED('sub-1'))
+        .mockRejectedValueOnce(new Error('Request timed out'))
+        .mockResolvedValueOnce(records([
+          { id: 'sub-1', emailId: 'e-1', undoStatus: 'canceled' },
+          { id: 'sub-2', emailId: 'e-1', undoStatus: 'final', sendAt: '2026-09-25T08:00:00Z' },
+        ]));
+
+      const result = await rescheduleScheduledSend(SCHEDULED, 0);
+
+      expect(result).toEqual({ emailSubmissionId: 'sub-2', sendAt: '2026-09-25T08:00:00Z' });
+      expect(creates()).toBe(1);
+      const [query] = mockRequest.mock.calls.at(-1)![0];
+      expect(query).toEqual(['EmailSubmission/query', { accountId: 'acc-1', filter: { emailIds: ['e-1'] } }, '0']);
+    });
+
+    it('puts the original back when the lost create left nothing on the server', async () => {
+      mockRequest
+        .mockResolvedValueOnce(LOOKUP)
+        .mockResolvedValueOnce(CANCELLED)
+        .mockResolvedValueOnce(DESTROYED('sub-1'))
+        .mockRejectedValueOnce(new Error('Request timed out'))
+        .mockResolvedValueOnce(records([{ id: 'sub-1', emailId: 'e-1', undoStatus: 'canceled' }]))
+        .mockResolvedValueOnce(set({ created: { replacement: { id: 'sub-3' } } }));
+
+      const err = await rescheduleScheduledSend(SCHEDULED, 7200).catch((e: unknown) => e);
+
+      expect(err).toBeInstanceOf(RescheduleRestoredError);
+      expect(err).toMatchObject({ restoredSubmissionId: 'sub-3' });
+    });
+
+    it('counts a put-back the server made for the answer that was lost as restored, and leaves the message in Sent', async () => {
+      mockRequest
+        .mockResolvedValueOnce(LOOKUP)
+        .mockResolvedValueOnce(CANCELLED)
+        .mockResolvedValueOnce(DESTROYED('sub-1'))
+        .mockResolvedValueOnce(REFUSED('Quota exceeded'))
+        .mockRejectedValueOnce(new Error('Network request failed'))
+        .mockResolvedValueOnce(records([{ id: 'sub-3', emailId: 'e-1', undoStatus: 'pending', sendAt: ORIGINAL_AT }]));
+
+      const err = await rescheduleScheduledSend(SCHEDULED, 7200).catch((e: unknown) => e);
+
+      expect(err).toBeInstanceOf(RescheduleRestoredError);
+      expect(err).toMatchObject({ restoredSubmissionId: 'sub-3', restoredSendAt: ORIGINAL_AT });
+      expect(mockRequest.mock.calls.some((c) => c[0][0][0] === 'Email/set')).toBe(false);
+    });
+  });
+
   // A group send's undo entry carried the login's own identity id, which
   // names nothing in the group account: both creates were refused and the
   // message went back to Drafts unsent.

src/api/__tests__/live-stalwart-compose.test.ts

@@ -22,7 +22,7 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
 
 import { JMAPClient } from '../jmap-client';
 import {
-  destroyEmails, getFullEmails, getMailboxes, listScheduledEmails, rescheduleScheduledSend, saveDraftVersion, sendEmail,
+  cancelScheduledSend, destroyEmails, getFullEmails, getMailboxes, listScheduledEmails, rescheduleScheduledSend, saveDraftVersion, sendEmail,
 } from '../email';
 import { uploadBytes } from '../blob';
 import { deleteFileNodes } from '../files';
@@ -69,6 +69,11 @@ live('live Stalwart: composer and sending (0.16 and 1.0)', () => {
   }, 60_000);
 
   afterAll(async () => {
+    // A failed test may leave a send scheduled; it must not outlive its message.
+    const held = await listScheduledEmails().catch(() => []);
+    for (const s of held.filter((h) => createdEmails.includes(h.emailId))) {
+      await cancelScheduledSend(s.emailSubmissionId).catch(() => undefined);
+    }
     const res = await singleton.request([['Email/query', { accountId: singleton.accountId, filter: { subject: tag } }, '0']]);
     const leftovers = [...new Set([...(res.methodResponses[0][1].ids as string[] ?? []), ...createdEmails])];
     if (leftovers.length) await destroyEmails(leftovers).catch(() => undefined);
@@ -115,6 +120,62 @@ live('live Stalwart: composer and sending (0.16 and 1.0)', () => {
     }, 90_000);
   });
 
+  describe('scheduled sends whose answers get lost', () => {
+    /** Let the first call matching `pick` reach the server, then lose its answer. */
+    const loseAnswerOf = (pick: (method: string, args: Record<string, unknown>) => boolean) => {
+      const real = singleton.request.bind(singleton);
+      let lost = false;
+      return vi.spyOn(singleton, 'request').mockImplementation(async (calls, using) => {
+        const res = await real(calls, using);
+        const [method, args] = calls[0] as unknown as [string, Record<string, unknown>];
+        if (!lost && pick(method, args)) {
+          lost = true;
+          throw new Error('Request timed out');
+        }
+        return res;
+      });
+    };
+    const scheduleOne = async (label: string) => {
+      const [identity] = await getIdentities();
+      const result = await sendEmail(
+        { from: [{ email: USER }], to: [{ email: USER }], subject: `${label} ${tag}`, textBody: 'live' },
+        identity.id, sent, 3600, { draftsMailboxId: drafts },
+      );
+      createdEmails.push(result.emailId!);
+      return (await listScheduledEmails()).find((s) => s.emailSubmissionId === result.emailSubmissionId)!;
+    };
+    const pendingOf = async (emailId: string) => (await listScheduledEmails()).filter((s) => s.emailId === emailId);
+
+    it('makes the replacement when the cancel went through and its answer was lost', async () => {
+      const first = await scheduleOne('lost cancel');
+      const spy = loseAnswerOf((method, args) => method === 'EmailSubmission/set' && !!args.update);
+      let moved: Awaited<ReturnType<typeof rescheduleScheduledSend>>;
+      try {
+        moved = await rescheduleScheduledSend(first, 7200);
+      } finally {
+        spy.mockRestore();
+      }
+      const pending = await pendingOf(first.emailId);
+      expect(pending.map((s) => s.emailSubmissionId)).toEqual([moved.emailSubmissionId]);
+      await cancelScheduledToDrafts(pending[0], await getMailboxes(), singleton.accountId);
+    }, 90_000);
+
+    it('takes the replacement the server made when the answer to its create was lost, and schedules it once', async () => {
+      const first = await scheduleOne('lost create');
+      const spy = loseAnswerOf((method, args) => method === 'EmailSubmission/set' && !!args.create);
+      let moved: Awaited<ReturnType<typeof rescheduleScheduledSend>>;
+      try {
+        moved = await rescheduleScheduledSend(first, 7200);
+      } finally {
+        spy.mockRestore();
+      }
+      const pending = await pendingOf(first.emailId);
+      expect(pending.map((s) => s.emailSubmissionId)).toEqual([moved.emailSubmissionId]);
+      expect(new Date(pending[0].sendAt).getTime()).toBeGreaterThan(new Date(first.sendAt).getTime() + 3_000_000);
+      await cancelScheduledToDrafts(pending[0], await getMailboxes(), singleton.accountId);
+    }, 90_000);
+  });
+
   describe('Files and mail share blobs', () => {
     it('saves an attachment to Files and attaches a file from Files, by blob', async () => {
       const upload = await uploadBytes(new TextEncoder().encode(`hello ${RUN}`), 'text/plain');

src/api/email.ts

@@ -1,6 +1,6 @@
 import { jmapClient } from './jmap-client';
 import {
-  assertSetResult, batched, failedIdsOf, JMAPSetError, NoLongerScheduledError, parseHoldLimit, RecipientsRefusedError,
+  assertSetResult, batched, failedIdsOf, JMAPMethodError, JMAPSetError, NoLongerScheduledError, parseHoldLimit, RecipientsRefusedError,
   requireMethodResult, ScheduleTooLateError, type RefusedRecipient,
 } from './jmap-result';
 import { keywordPointer, mailboxPointer } from './patch-pointer';
@@ -2146,7 +2146,8 @@ export async function cancelScheduledSend(emailSubmissionId: string, accountIdOv
     | { type?: string; description?: string }
     | undefined;
   if (failure) {
-    throw new Error(failure.description ?? failure.type ?? 'Failed to cancel scheduled send');
+    // A SetError: the server answered and kept the submission as it was.
+    throw new JMAPSetError(failure.type ?? 'setError', failure.description ?? failure.type ?? 'Failed to cancel scheduled send');
   }
   // The canceled record is dead weight against Stalwart 1.0's submission
   // limit. It goes in a request of its own, and only once the cancel is
@@ -2201,9 +2202,10 @@ export class RescheduleLostError extends Error {
  * out already (NoLongerScheduledError). A replacement the server refuses, or
  * takes for no recipient, puts the original back at its old time under a new
  * id (RescheduleRestoredError); when that fails too the message goes back to
- * Drafts (RescheduleLostError). Returns the new submission id and send time,
- * and the recipients the server refused for the replacement when it took
- * the others.
+ * Drafts (RescheduleLostError). A cancel or create whose answer was lost is
+ * checked against the server's records first. Returns the new submission id
+ * and send time, and the recipients the server refused for the replacement
+ * when it took the others.
  */
 export async function rescheduleScheduledSend(
   scheduled: {
@@ -2263,6 +2265,9 @@ export async function rescheduleScheduledSend(
   const sentId = roles.find((m) => m.role === 'sent')?.id;
 
   type Created = { id: string; sendAt?: string; delivery: { recipients: RefusedRecipient[]; all: boolean } };
+  // Creates that got no answer (timeout, dropped connection): the server
+  // may have taken them all the same.
+  const unanswered = new Set<unknown>();
   // `until`: when the submission goes out (ms); undefined, at once.
   const createSubmission = async (until: number | undefined): Promise<Created> => {
     // HOLDFOR counts from when the server takes the request, so a retry
@@ -2295,7 +2300,10 @@ export async function rescheduleScheduledSend(
           deliveryStatusCall(accountId, 'replacement', '1'),
         ],
         SUBMISSION_USING,
-      );
+      ).catch((err: unknown) => {
+        unanswered.add(err);
+        throw err;
+      });
     };
     let res = await send();
     // A full submission quota (Stalwart 1.0): make room and try once more.
@@ -2336,8 +2344,17 @@ export async function rescheduleScheduledSend(
     await cancelScheduledSend(scheduled.emailSubmissionId, accountId);
   } catch (err) {
     const status = await submissionUndoStatus(scheduled.emailSubmissionId, accountId);
-    if (status === 'final' || status === 'canceled') throw new NoLongerScheduledError(status === 'final');
-    throw err;
+    // A cancel the server answered and refused changed nothing, so a
+    // canceled original was cancelled elsewhere. One whose answer was lost
+    // (timeout, dropped connection) may have gone through: then the
+    // original is cancelled by this call and the replacement still has to
+    // be made, or the message sits in Sent unsent.
+    const refusedCancel = err instanceof JMAPMethodError;
+    if (status === 'final' || (status === 'canceled' && refusedCancel)) {
+      throw new NoLongerScheduledError(status === 'final');
+    }
+    if (status !== 'canceled') throw err;
+    await destroyEmailSubmissions([scheduled.emailSubmissionId], accountId);
   }
 
   let replacement: Created;
@@ -2350,6 +2367,13 @@ export async function rescheduleScheduledSend(
     const refused = await withdrawIfDead(replacement);
     if (refused) throw new RecipientsRefusedError(refused);
   } catch (err) {
+    // A replacement whose answer was lost may have been taken: putting the
+    // original back as well would send the message twice. The server's
+    // record of it stands in for the answer.
+    if (unanswered.has(err)) {
+      const taken = await liveSubmissionOf(scheduled.emailId, accountId);
+      if (taken) return { emailSubmissionId: taken.id, sendAt: taken.sendAt };
+    }
     // Put the original back at its old time: HOLDFOR with the seconds left,
     // at once when that time has passed meanwhile.
     const originalTime = original?.sendAt ? Date.parse(original.sendAt) : NaN;
@@ -2357,7 +2381,9 @@ export async function rescheduleScheduledSend(
     try {
       restored = await createSubmission(Number.isFinite(originalTime) && originalTime > Date.now() ? originalTime : undefined);
     } catch (restoreErr) {
-      console.warn('[email] could not put the original scheduled send back:', restoreErr);
+      const taken = unanswered.has(restoreErr) ? await liveSubmissionOf(scheduled.emailId, accountId) : undefined;
+      if (taken) restored = { id: taken.id, sendAt: taken.sendAt, delivery: { recipients: [], all: false } };
+      else console.warn('[email] could not put the original scheduled send back:', restoreErr);
     }
     if (restored && !(await withdrawIfDead(restored).catch(() => null))) {
       throw new RescheduleRestoredError(err, restored.id, restored.sendAt ?? original?.sendAt);
@@ -2396,6 +2422,33 @@ async function readSubmissionUndoStatus(emailSubmissionId: string, accountId: st
   return list?.find((s) => s.id === emailSubmissionId)?.undoStatus;
 }
 
+/**
+ * A submission of `emailId` that is still held or went out, for a create
+ * whose answer was lost. Undefined when there is none or it can't be read.
+ */
+async function liveSubmissionOf(emailId: string, accountId: string): Promise<{ id: string; sendAt?: string } | undefined> {
+  try {
+    const res = await jmapClient.request(
+      [
+        ['EmailSubmission/query', { accountId, filter: { emailIds: [emailId] } }, '0'],
+        ['EmailSubmission/get', {
+          accountId,
+          '#ids': { resultOf: '0', name: 'EmailSubmission/query', path: '/ids' },
+          properties: ['id', 'emailId', 'undoStatus', 'sendAt'],
+        }, '1'],
+      ],
+      SUBMISSION_ONLY_USING,
+    );
+    const list = requireMethodResult(res, '1', 'EmailSubmission/get').list as
+      Array<{ id?: string; emailId?: string; undoStatus?: string; sendAt?: string }> | undefined;
+    // "Send now" goes out a second later: a final one is the replacement too.
+    const live = list?.find((s) => s.id && s.emailId === emailId && (s.undoStatus === 'pending' || s.undoStatus === 'final'));
+    return live?.id ? { id: live.id, sendAt: live.sendAt } : undefined;
+  } catch {
+    return undefined;
+  }
+}
+
 /** A submission's `undoStatus`, or undefined when it can't be read. */
 async function submissionUndoStatus(emailSubmissionId: string, accountId: string): Promise<string | undefined> {
   try {