fix: spend a plugin's undo token only once its undo went through, so a failed undo can be tried again

lib/__tests__/plugin-assistant-undo-tokens.test.ts

@@ -0,0 +1,49 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { UNDO_TOKEN_TTL_MS, clearUndoTokens, issueUndoToken, withUndoToken } from '../plugin-sandbox/assistant/c.undo';
+
+// The host keeps what an undo needs and hands the plugin only a token: bound
+// to that plugin and account, good for 30 minutes, spent once the undo went
+// through, kept when it failed.
+
+const use = (token: unknown, { plugin = 'p1', account = 'acc1', kind = 'batch' } = {}, run = async (data: unknown) => data) =>
+  withUndoToken('test.undo', plugin, account, kind, token, run);
+
+beforeEach(() => { clearUndoTokens(); });
+afterEach(() => { vi.useRealTimers(); });
+
+describe('undo tokens', () => {
+  it('work for 30 minutes and not after', async () => {
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date('2026-11-03T10:00:00Z'));
+    const early = issueUndoToken('p1', 'acc1', 'batch', 'a');
+    const late = issueUndoToken('p1', 'acc1', 'batch', 'b');
+    vi.setSystemTime(Date.now() + UNDO_TOKEN_TTL_MS - 1000);
+    expect(await use(early)).toBe('a');
+    vi.setSystemTime(Date.now() + 2000);
+    await expect(use(late)).rejects.toThrow(/can't be undone any more/);
+  });
+
+  it('belong to the plugin, the account and the kind they were made for', async () => {
+    const token = issueUndoToken('p1', 'acc1', 'batch', 'x');
+    await expect(use(token, { account: 'acc2' })).rejects.toThrow(/can't be undone any more/);
+    await expect(use(token, { plugin: 'p2' })).rejects.toThrow(/can't be undone any more/);
+    await expect(use(token, { kind: 'filters' })).rejects.toThrow(/can't be undone any more/);
+    expect(await use(token)).toBe('x');
+  });
+
+  it('are spent when the undo went through, and kept when it failed', async () => {
+    const token = issueUndoToken('p1', 'acc1', 'batch', 'x');
+    await expect(use(token, {}, async () => { throw new Error('server down'); })).rejects.toThrow('server down');
+    expect(await use(token)).toBe('x');
+    await expect(use(token)).rejects.toThrow(/can't be undone any more/);
+  });
+
+  it('run one undo at a time', async () => {
+    const token = issueUndoToken('p1', 'acc1', 'batch', 'x');
+    let finish!: () => void;
+    const first = use(token, {}, () => new Promise((resolve) => { finish = () => resolve('done'); }));
+    await expect(use(token)).rejects.toThrow(/can't be undone any more/);
+    finish();
+    expect(await first).toBe('done');
+  });
+});

lib/__tests__/plugin-host-batch.test.ts

@@ -129,6 +129,15 @@ describe('api.email.batch', () => {
     expect(store.batchMoveToMailbox).not.toHaveBeenCalled();
   });
 
+  it('lets the plugin try a failed undo again', async () => {
+    const { undoToken } = await call(WRITER, 'email.batch', { emailIds: ['m1'], action: 'archive' }) as { undoToken: string };
+    store.undoMove.mockRejectedValueOnce(new Error('server down'));
+    await expect(call(WRITER, 'email.undoBatch', undoToken)).rejects.toThrow('server down');
+    await call(WRITER, 'email.undoBatch', undoToken);
+    expect(store.undoMove).toHaveBeenCalledTimes(2);
+    await expect(call(WRITER, 'email.undoBatch', undoToken)).rejects.toThrow(/can't be undone/);
+  });
+
   it('keeps the undo to the plugin that made it, and needs email:write', async () => {
     const { undoToken } = await call(WRITER, 'email.batch', { emailIds: ['m1'], action: 'archive' }) as { undoToken: string };
     await expect(call(plugin('other', ['email:write']), 'email.undoBatch', undoToken)).rejects.toThrow(/can't be undone/);

lib/plugin-sandbox/assistant/c.batch.ts

@@ -14,7 +14,7 @@ import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from '../../thread-utils';
 import { readId, readObject, readString, sessionClient, withinRate } from '../host-api-input';
 import { MAX_PLUGIN_MOVES_PER_MINUTE } from '../host-api-mail';
 import { isSharedOut } from '../../mailbox-shared-out';
-import { issueUndoToken, redeemUndoToken, undoAccount } from './c.undo';
+import { issueUndoToken, undoAccount, withUndoToken } from './c.undo';
 import type { MailBatchAction, MailBatchResult } from './c.types';
 
 /** Most messages one email.batch call changes. */
@@ -28,9 +28,13 @@ interface BatchUndo {
   run: () => Promise<void>;
 }
 
+/** Runs `fn` once while it runs or after it went through; a failure may be tried again. */
 function once(fn: () => Promise<void>): () => Promise<void> {
   let promise: Promise<void> | null = null;
-  return () => (promise ??= fn());
+  return () => (promise ??= fn().catch((err: unknown) => {
+    promise = null;
+    throw err;
+  }));
 }
 
 function readIds(value: unknown, method: string): string[] {
@@ -145,9 +149,10 @@ export async function doEmailBatch(pluginId: string, input: unknown): Promise<Ma
 export async function doEmailUndoBatch(pluginId: string, token: unknown): Promise<void> {
   const method = 'email.undoBatch';
   const client: IJMAPClient = sessionClient(method);
-  const { entryId, run } = redeemUndoToken<BatchUndo>(method, pluginId, undoAccount(client), 'batch', token);
-  // Off the history first, so Ctrl+Z can't run it again; an entry already
-  // taken by Ctrl+Z has run (or is running) the same undo.
-  runUndo(entryId);
-  await run();
+  await withUndoToken<BatchUndo, void>(method, pluginId, undoAccount(client), 'batch', token, async ({ entryId, run }) => {
+    // Off the history first, so Ctrl+Z can't run it again; an entry already
+    // taken by Ctrl+Z has run (or is running) the same undo.
+    runUndo(entryId);
+    await run();
+  });
 }

lib/plugin-sandbox/assistant/c.calendar.ts

@@ -12,7 +12,7 @@ import { answerInvitation, attendeeOf, readOwnInvitation, type RsvpStatus } from
 import { readId, sessionClient } from '../host-api-input';
 import { readNamedEmail } from '../host-api-mail';
 import { doCalendarDeleteEvent } from '../host-api-calendar';
-import { issueUndoToken, redeemUndoToken, undoAccount } from './c.undo';
+import { issueUndoToken, undoAccount, withUndoToken } from './c.undo';
 import type { EventDeleteResult, InvitationResponseView } from './c.types';
 
 const STATUSES: readonly RsvpStatus[] = ['accepted', 'tentative', 'declined'];
@@ -161,11 +161,17 @@ export async function doCalendarDeleteWithUndo(pluginId: string, idArg: unknown)
   return { undoToken: issueUndoToken(pluginId, undoAccount(client), 'event', copy) };
 }
 
-/** Puts back an event calendar.deleteEvent deleted, from the host's copy. Sends no invitations. */
-export async function doCalendarUndoDelete(pluginId: string, tokenArg: unknown): Promise<{ id: string } | null> {
+/**
+ * Puts back an event calendar.deleteEvent deleted, from the host's copy.
+ * Sends no invitations. When it can't be created (another plugin stopped
+ * it, the server refused) the token stays for another try.
+ */
+export async function doCalendarUndoDelete(pluginId: string, tokenArg: unknown): Promise<{ id: string }> {
   const method = 'calendar.undoDelete';
   const client = sessionClient(method);
-  const copy = redeemUndoToken<Partial<CalendarEvent>>(method, pluginId, undoAccount(client), 'event', tokenArg);
-  const created = await useCalendarStore.getState().createEvent(client, copy, false);
-  return created ? { id: created.id } : null;
+  return withUndoToken<Partial<CalendarEvent>, { id: string }>(method, pluginId, undoAccount(client), 'event', tokenArg, async (copy) => {
+    const created = await useCalendarStore.getState().createEvent(client, copy, false);
+    if (!created) throw new Error(`${method}: the event was not created again`);
+    return { id: created.id };
+  });
 }

lib/plugin-sandbox/assistant/c.filters.ts

@@ -21,7 +21,7 @@ import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from '../../thread-utils';
 import { generateUUID } from '../../utils';
 import { isPlainRecord, readId, readObject, readString, sessionClient } from '../host-api-input';
 import { isSharedOut } from '../../mailbox-shared-out';
-import { issueUndoToken, redeemUndoToken, undoAccount } from './c.undo';
+import { issueUndoToken, undoAccount, withUndoToken } from './c.undo';
 import type {
   FilterCreateResult, FilterRuleActionType, FilterRuleField, FilterRuleInfo, FilterRuleOp, FiltersListView,
 } from './c.types';
@@ -231,12 +231,13 @@ export async function doFiltersCreate(pluginId: string, input: unknown): Promise
 export async function doFiltersUndo(pluginId: string, token: unknown): Promise<void> {
   const method = 'filters.undo';
   const client = sieveClient(method);
-  const change = redeemUndoToken<FiltersChange>(method, pluginId, undoAccount(client), 'filters', token);
-  try {
-    await restoreAccountFilters(client, change);
-  } catch (err) {
-    if (err instanceof FiltersChangedError) throw new Error(`${method}: the filters changed since; change them in Settings > Filters`);
-    throw err;
-  }
-  refreshFilterStore(client, change.accountId);
+  await withUndoToken<FiltersChange, void>(method, pluginId, undoAccount(client), 'filters', token, async (change) => {
+    try {
+      await restoreAccountFilters(client, change);
+    } catch (err) {
+      if (err instanceof FiltersChangedError) throw new Error(`${method}: the filters changed since; change them in Settings > Filters`);
+      throw err;
+    }
+    refreshFilterStore(client, change.accountId);
+  });
 }

lib/plugin-sandbox/assistant/c.settings.ts

@@ -26,7 +26,7 @@ import {
 } from '../../settings-schema';
 import { isPlainRecord, sessionClient } from '../host-api-input';
 import { messagesFor } from '../host-api-app';
-import { issueUndoToken, redeemUndoToken, undoAccount } from './c.undo';
+import { issueUndoToken, undoAccount, withUndoToken } from './c.undo';
 import type {
   SettingDescription, SettingRefusal, SettingValue, SettingsChangeResult, SettingsUndoResult,
 } from './c.types';
@@ -268,7 +268,11 @@ export async function doSetSettings(pluginId: string, arg: unknown): Promise<Set
 export async function doUndoSettings(pluginId: string, token: unknown): Promise<SettingsUndoResult> {
   const method = 'app.undoSettings';
   const client = sessionClient(method);
-  const previous = redeemUndoToken<Record<string, SettingValue | null>>(method, pluginId, undoAccount(client), 'settings', token);
+  return withUndoToken<Record<string, SettingValue | null>, SettingsUndoResult>(method, pluginId, undoAccount(client), 'settings', token, (previous) => restoreSettings(previous));
+}
+
+/** Writes the old values back, setting by setting (see doUndoSettings). */
+async function restoreSettings(previous: Record<string, SettingValue | null>): Promise<SettingsUndoResult> {
   const restored: string[] = [];
   const refused: SettingsUndoResult['refused'] = {};
   for (const [key, old] of Object.entries(previous)) {

lib/plugin-sandbox/assistant/c.undo.ts

@@ -19,6 +19,8 @@ interface Entry {
   kind: string;
   expires: number;
   data: unknown;
+  /** Its undo is running: a second call waits for nothing and is refused. */
+  busy?: boolean;
 }
 
 const tokens = new Map<string, Entry>();
@@ -39,18 +41,28 @@ export function issueUndoToken(pluginId: string, accountId: string, kind: string
 }
 
 /**
- * Takes the data of a token: only for the plugin that got it, in the same
- * account, for the same kind, before it ran out. A token works once.
+ * Runs the undo a token stands for: only for the plugin that got it, in the
+ * same account, for the same kind, before it ran out. The token is spent
+ * when the undo went through; when it failed the token stays, so the plugin
+ * can try again. One undo at a time per token.
  */
-export function redeemUndoToken<T>(method: string, pluginId: string, accountId: string, kind: string, token: unknown): T {
+export async function withUndoToken<T, R>(
+  method: string, pluginId: string, accountId: string, kind: string, token: unknown, run: (data: T) => Promise<R>,
+): Promise<R> {
   if (typeof token !== 'string' || !token) throw new Error(`${method}: undoToken must be a string`);
+  // Expired tokens are dropped when new ones are made; the time is checked here.
   const entry = tokens.get(token);
-  prune(Date.now());
-  if (!entry || entry.expires <= Date.now() || entry.pluginId !== pluginId || entry.accountId !== accountId || entry.kind !== kind) {
+  if (!entry || entry.busy || entry.expires <= Date.now() || entry.pluginId !== pluginId || entry.accountId !== accountId || entry.kind !== kind) {
     throw new Error(`${method}: this can't be undone any more`);
   }
-  tokens.delete(token);
-  return entry.data as T;
+  entry.busy = true;
+  try {
+    const result = await run(entry.data as T);
+    tokens.delete(token);
+    return result;
+  } finally {
+    entry.busy = false;
+  }
 }
 
 /** The account an undo token is bound to: the client's JMAP account, else the signed-in login. */