fix: stop a failed reschedule from dropping or double-sending the message

Rescheduling cancelled the original submission and created its
replacement in one `EmailSubmission/set`, whose create and update the
server applies independently. A refused replacement (a hold over the
server's limit, a quota) still cancelled the original, so the message
silently never went out, and a refused cancel (the original had just
gone out) still let the replacement go, so it was sent twice.

Ported the webmail's order:
- the replacement is created first; if it is refused, the original
  stays scheduled
- the original is cancelled only once the replacement exists
- if the original can't be cancelled, the replacement is withdrawn
  again, unless a cancel whose answer got lost did go through
- "Send now" is a 1-second hold, so the replacement can still be
  withdrawn

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

@@ -31,10 +31,27 @@ const SCHEDULED = {
   to: [{ email: 'to@example.com' }],
 };
 
+const LOOKUP = {
+  methodResponses: [
+    ['EmailSubmission/get', {
+      list: [{ id: 'sub-1', envelope: { mailFrom: { email: 'me@example.com' }, rcptTo: [{ email: 'to@example.com' }] } }],
+    }, '0'],
+    ['Email/get', { list: [{ id: 'e-1', to: [{ email: 'to@example.com' }] }] }, '1'],
+  ],
+};
 const REPLACED = [['EmailSubmission/set', {
-  updated: { 'sub-1': null },
   created: { replacement: { id: 'sub-2', sendAt: '2026-09-25T08:00:00Z' } },
 }, '0']];
+const set = (body: Record<string, unknown>) => ({ methodResponses: [['EmailSubmission/set', body, '0']] });
+const CANCELLED = set({ updated: { 'sub-1': null } });
+/** The EmailSubmission/set calls made, as [create ids, update ids]. */
+const setCalls = () => mockRequest.mock.calls
+  .map((c) => c[0][0])
+  .filter(([method]: [string]) => method === 'EmailSubmission/set')
+  .map(([, args]: [string, { create?: object; update?: Record<string, unknown> }]) => [
+    Object.keys(args.create ?? {}),
+    Object.keys(args.update ?? {}),
+  ]);
 
 describe('rescheduleScheduledSend', () => {
   it('keeps Cc, Bcc and the envelope sender of the held submission (B19)', async () => {
@@ -57,7 +74,8 @@ describe('rescheduleScheduledSend', () => {
           ['Email/get', { list: [{ id: 'e-1', to: [{ email: 'to@example.com' }], cc: [{ email: 'cc@example.com' }] }] }, '1'],
         ],
       })
-      .mockResolvedValueOnce({ methodResponses: REPLACED });
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(CANCELLED);
 
     const result = await rescheduleScheduledSend(SCHEDULED, 7200);
 
@@ -65,7 +83,6 @@ describe('rescheduleScheduledSend', () => {
     const [lookup] = mockRequest.mock.calls[0][0];
     expect(lookup).toEqual(['EmailSubmission/get', { accountId: 'acc-1', ids: ['sub-1'], properties: ['envelope'] }, '0']);
     const args = mockRequest.mock.calls[1][0][0][1];
-    expect(args.update).toEqual({ 'sub-1': { undoStatus: 'canceled' } });
     expect(args.create.replacement.envelope).toEqual({
       mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '7200' } },
       rcptTo: [{ email: 'to@example.com' }, { email: 'cc@example.com' }, { email: 'bcc@example.com' }],
@@ -87,7 +104,8 @@ describe('rescheduleScheduledSend', () => {
           }, '1'],
         ],
       })
-      .mockResolvedValueOnce({ methodResponses: REPLACED });
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(CANCELLED);
 
     await rescheduleScheduledSend(SCHEDULED, 60);
 
@@ -97,13 +115,84 @@ describe('rescheduleScheduledSend', () => {
     });
   });
 
-  it('sends now without a lookup and lets the server derive the envelope', async () => {
+  it('creates the replacement before cancelling the original', async () => {
-    mockRequest.mockResolvedValueOnce({ methodResponses: REPLACED });
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(CANCELLED);
+
+    await rescheduleScheduledSend(SCHEDULED, 7200);
+
+    expect(setCalls()).toEqual([[['replacement'], []], [[], ['sub-1']]]);
+    expect(mockRequest.mock.calls[2][0][0][1].update).toEqual({ 'sub-1': { undoStatus: 'canceled' } });
+  });
+
+  it('sends now as a 1-second hold, so the replacement can still be withdrawn', async () => {
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(CANCELLED);
 
     await rescheduleScheduledSend(SCHEDULED, 0);
 
-    expect(mockRequest).toHaveBeenCalledTimes(1);
+    const replacement = mockRequest.mock.calls[1][0][0][1].create.replacement;
-    expect(mockRequest.mock.calls[0][0][0][1].create.replacement).toEqual({ emailId: 'e-1', identityId: 'id-1' });
+    expect(replacement.envelope).toEqual({
+      mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '1' } },
+      rcptTo: [{ email: 'to@example.com' }],
+    });
+    expect(setCalls()).toEqual([[['replacement'], []], [[], ['sub-1']]]);
+  });
+
+  it('leaves the original scheduled when the replacement is refused', async () => {
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce(set({ notCreated: { replacement: { type: 'forbiddenToSend', description: 'Quota exceeded' } } }));
+
+    await expect(rescheduleScheduledSend(SCHEDULED, 7200)).rejects.toThrow('Quota exceeded');
+    // No cancel went out.
+    expect(setCalls()).toEqual([[['replacement'], []]]);
+  });
+
+  it('withdraws the replacement when the original has already gone out', async () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(set({ notUpdated: { 'sub-1': { type: 'cannotUnsend', description: 'Already sent' } } }))
+      .mockResolvedValueOnce({ methodResponses: [['EmailSubmission/get', { list: [{ id: 'sub-1', undoStatus: 'final' }] }, '0']] })
+      .mockResolvedValueOnce(set({ updated: { 'sub-2': null } }));
+
+    await expect(rescheduleScheduledSend(SCHEDULED, 0)).rejects.toThrow('Failed to cancel the previous schedule: Already sent');
+    expect(setCalls()).toEqual([[['replacement'], []], [[], ['sub-1']], [[], ['sub-2']]]);
+    expect(warn).not.toHaveBeenCalled();
+    warn.mockRestore();
+  });
+
+  it('still reports the failure when the replacement cannot be withdrawn either', async () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockResolvedValueOnce(set({ notUpdated: { 'sub-1': { type: 'cannotUnsend' } } }))
+      .mockResolvedValueOnce({ methodResponses: [['EmailSubmission/get', { list: [{ id: 'sub-1', undoStatus: 'final' }] }, '0']] })
+      .mockResolvedValueOnce(set({ notUpdated: { 'sub-2': { type: 'cannotUnsend' } } }));
+
+    await expect(rescheduleScheduledSend(SCHEDULED, 0)).rejects.toThrow('Failed to cancel the previous schedule');
+    expect(warn).toHaveBeenCalledTimes(1);
+    warn.mockRestore();
+  });
+
+  it('keeps the replacement when a cancel whose answer got lost did go through', async () => {
+    mockRequest
+      .mockResolvedValueOnce(LOOKUP)
+      .mockResolvedValueOnce({ methodResponses: REPLACED })
+      .mockRejectedValueOnce(new Error('Network request failed'))
+      .mockResolvedValueOnce({ methodResponses: [['EmailSubmission/get', { list: [{ id: 'sub-1', undoStatus: 'canceled' }] }, '0']] });
+
+    const result = await rescheduleScheduledSend(SCHEDULED, 7200);
+
+    expect(result.emailSubmissionId).toBe('sub-2');
+    expect(setCalls()).toEqual([[['replacement'], []], [[], ['sub-1']]]);
   });
 });
 
@@ -216,13 +305,15 @@ describe('hold-limit rejections', () => {
         ],
       })
       .mockResolvedValueOnce({
-        methodResponses: [['EmailSubmission/set', { updated: { 'sub-1': null }, notCreated: { replacement: HOLD_REJECTED } }, '0']],
+        methodResponses: [['EmailSubmission/set', { notCreated: { replacement: HOLD_REJECTED } }, '0']],
       });
 
     const err = await rescheduleScheduledSend(SCHEDULED, 5 * 24 * 3600).catch((e) => e);
 
     expect(err).toBeInstanceOf(ScheduleTooLateError);
     expect(jmapClient.learnHoldLimit).toHaveBeenCalledWith(172_800);
+    // The original stays scheduled.
+    expect(setCalls()).toEqual([[['replacement'], []]]);
   });
 });
 
@@ -301,13 +392,11 @@ describe('scheduled sends in shared accounts (webmail #874)', () => {
     await cancelScheduledSend('s-grp', 'grp-1');
     expect(mockRequest.mock.calls[0][0][0][1].accountId).toBe('grp-1');
 
-    mockRequest.mockResolvedValueOnce({
+    mockRequest
-      methodResponses: [['EmailSubmission/set', {
+      .mockResolvedValueOnce(LOOKUP)
-        updated: { 's-grp': null },
+      .mockResolvedValueOnce(set({ created: { replacement: { id: 's-new', sendAt: FUTURE } } }))
-        created: { replacement: { id: 's-new', sendAt: FUTURE } },
+      .mockResolvedValueOnce(set({ updated: { 's-grp': null } }));
-      }, '0']],
-    });
     await rescheduleScheduledSend({ ...SCHEDULED, emailSubmissionId: 's-grp', accountId: 'grp-1' }, 0);
-    expect(mockRequest.mock.calls[1][0][0][1].accountId).toBe('grp-1');
+    expect(mockRequest.mock.calls.slice(1).map((c) => c[0][0][1].accountId)).toEqual(['grp-1', 'grp-1', 'grp-1']);
   });
 });

src/api/email.ts

@@ -1531,10 +1531,16 @@ export async function cancelScheduledSend(emailSubmissionId: string, accountIdOv
 }
 
 /**
- * Change when a scheduled message goes out (or send it now with `holdForSeconds`
+ * Change when a scheduled message goes out, or send it now with
- * = 0): cancel the pending submission and create a replacement for the same
+ * `holdForSeconds` = 0. A replacement submission for the same Email, with the
- * Email with the held envelope and a fresh HOLDFOR, in one request. Returns the new
+ * held envelope and a fresh HOLDFOR, is created first, and the original is
- * submission id and resolved send time.
+ * cancelled only once the replacement exists (webmail parity). Both used to
+ * go in one /set, whose create and update the server applies independently:
+ * a refused replacement still cancelled the original, so the message never
+ * went out, and a refused cancel (the original had just gone out) still let
+ * the replacement go, so it was sent twice. Now a refused replacement leaves
+ * the original alone, and a replacement whose original can't be cancelled is
+ * withdrawn again. Returns the new submission id and send time.
  */
 export async function rescheduleScheduledSend(
   scheduled: {
@@ -1550,49 +1556,79 @@ export async function rescheduleScheduledSend(
   recipients?: EmailAddress[],
 ): Promise<{ emailSubmissionId?: string; sendAt?: string }> {
   const accountId = scheduled.accountId ?? jmapClient.accountId;
-  const create: Record<string, unknown> = { emailId: scheduled.emailId, identityId: scheduled.identityId };
+  // "Send now" is a 1-second hold, so the replacement can still be withdrawn.
-  if (holdForSeconds > 0) {
+  const holdFor = Math.max(1, Math.ceil(holdForSeconds));
-    // Reuse the held submission's envelope: it names every recipient,
+  // Reuse the held submission's envelope: it names every recipient,
-    // Cc and Bcc included, and the envelope sender a catch-all From went
+  // Cc and Bcc included, and the envelope sender a catch-all From went
-    // out through. Rebuilding it from the Email's To dropped Cc and Bcc.
+  // out through. Rebuilding it from the Email's To dropped Cc and Bcc.
-    // The Email's To/Cc/Bcc are the fallback when the server has none.
+  // The Email's To/Cc/Bcc are the fallback when the server has none.
-    const lookup = await jmapClient.request(
+  const lookup = await jmapClient.request(
-      [
+    [
-        ['EmailSubmission/get', { accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope'] }, '0'],
+      ['EmailSubmission/get', { accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope'] }, '0'],
-        ['Email/get', { accountId, ids: [scheduled.emailId], properties: ['to', 'cc', 'bcc'] }, '1'],
+      ['Email/get', { accountId, ids: [scheduled.emailId], properties: ['to', 'cc', 'bcc'] }, '1'],
-      ],
+    ],
-      [CAPABILITIES.CORE, CAPABILITIES.MAIL, CAPABILITIES.SUBMISSION],
+    [CAPABILITIES.CORE, CAPABILITIES.MAIL, CAPABILITIES.SUBMISSION],
-    );
+  );
-    const envelope = (requireMethodResult(lookup, '0', 'EmailSubmission/get').list as Array<{
+  const envelope = (requireMethodResult(lookup, '0', 'EmailSubmission/get').list as Array<{
-      envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null;
+    envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null;
-    }> | undefined)?.[0]?.envelope;
+  }> | undefined)?.[0]?.envelope;
-    const held = requireMethodResult(lookup, '1', 'Email/get').list as Email[] | undefined;
+  const held = requireMethodResult(lookup, '1', 'Email/get').list as Email[] | undefined;
-    const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])];
+  const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])];
-    const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients);
+  const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients);
-    const rcpt = source.map((r) => ({ email: (r.email ?? '').trim() })).filter((r) => r.email);
+  const rcpt = source.map((r) => ({ email: (r.email ?? '').trim() })).filter((r) => r.email);
-    create.envelope = {
+  const create = {
+    emailId: scheduled.emailId,
+    identityId: scheduled.identityId,
+    envelope: {
       mailFrom: {
         email: envelope?.mailFrom?.email || scheduled.from?.[0]?.email,
-        parameters: { HOLDFOR: String(Math.ceil(holdForSeconds)) },
+        parameters: { HOLDFOR: String(holdFor) },
       },
       rcptTo: rcpt,
-    };
+    },
-  }
+  };
+
   const res = await jmapClient.request(
-    [['EmailSubmission/set', {
+    [['EmailSubmission/set', { accountId, create: { replacement: create } }, '0']],
-      accountId,
-      update: { [scheduled.emailSubmissionId]: { undoStatus: 'canceled' } },
-      create: { replacement: create },
-    }, '0']],
     [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
   );
   const body = requireMethodResult(res, '0', 'EmailSubmission/set');
-  const notUpdated = body.notUpdated?.[scheduled.emailSubmissionId] as { description?: string; type?: string } | undefined;
-  if (notUpdated) throw new Error(notUpdated.description ?? notUpdated.type ?? 'Failed to cancel the previous schedule');
   const notCreated = body.notCreated?.replacement as { description?: string; type?: string } | undefined;
   if (notCreated) throw submissionError(notCreated, 'Failed to reschedule');
   const created = body.created?.replacement as { id?: string; sendAt?: string } | undefined;
-  return { emailSubmissionId: created?.id, sendAt: created?.sendAt };
+  if (!created?.id) throw new Error('Failed to reschedule: the server returned no submission');
+
+  try {
+    await cancelScheduledSend(scheduled.emailSubmissionId, accountId);
+  } catch (err) {
+    // A cancel whose answer got lost may still have gone through. Otherwise
+    // the original is still due or has just gone out, and the replacement
+    // must not go as well.
+    if ((await submissionUndoStatus(scheduled.emailSubmissionId, accountId)) !== 'canceled') {
+      try {
+        await cancelScheduledSend(created.id, accountId);
+      } catch (withdrawErr) {
+        console.warn('[email] could not withdraw the replacement scheduled send:', withdrawErr);
+      }
+      const reason = err instanceof Error ? err.message : String(err);
+      throw new Error(`Failed to cancel the previous schedule: ${reason}`);
+    }
+  }
+  return { emailSubmissionId: created.id, sendAt: created.sendAt };
+}
+
+/** A submission's `undoStatus`, or undefined when it can't be read. */
+async function submissionUndoStatus(emailSubmissionId: string, accountId: string): Promise<string | undefined> {
+  try {
+    const res = await jmapClient.request(
+      [['EmailSubmission/get', { accountId, ids: [emailSubmissionId], properties: ['undoStatus'] }, '0']],
+      [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION],
+    );
+    const list = requireMethodResult(res, '0', 'EmailSubmission/get').list as Array<{ undoStatus?: string }> | undefined;
+    return list?.[0]?.undoStatus;
+  } catch {
+    return undefined;
+  }
 }
 
 /**