diff --git a/lib/__tests__/plugin-assistant-undo-tokens.test.ts b/lib/__tests__/plugin-assistant-undo-tokens.test.ts new file mode 100644 index 000000000..946983774 --- /dev/null +++ b/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'); + }); +}); diff --git a/lib/__tests__/plugin-host-batch.test.ts b/lib/__tests__/plugin-host-batch.test.ts index 4ad4b63a5..a30f45fa9 100644 --- a/lib/__tests__/plugin-host-batch.test.ts +++ b/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/); diff --git a/lib/plugin-sandbox/assistant/c.batch.ts b/lib/plugin-sandbox/assistant/c.batch.ts index c6705bda3..15061edf0 100644 --- a/lib/plugin-sandbox/assistant/c.batch.ts +++ b/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; } +/** Runs `fn` once while it runs or after it went through; a failure may be tried again. */ function once(fn: () => Promise): () => Promise { let promise: Promise | 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 { const method = 'email.undoBatch'; const client: IJMAPClient = sessionClient(method); - const { entryId, run } = redeemUndoToken(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(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(); + }); } diff --git a/lib/plugin-sandbox/assistant/c.calendar.ts b/lib/plugin-sandbox/assistant/c.calendar.ts index 4fb58a8f0..9605c6565 100644 --- a/lib/plugin-sandbox/assistant/c.calendar.ts +++ b/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>(method, pluginId, undoAccount(client), 'event', tokenArg); - const created = await useCalendarStore.getState().createEvent(client, copy, false); - return created ? { id: created.id } : null; + return withUndoToken, { 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 }; + }); } diff --git a/lib/plugin-sandbox/assistant/c.filters.ts b/lib/plugin-sandbox/assistant/c.filters.ts index f6c856722..c1cc035fe 100644 --- a/lib/plugin-sandbox/assistant/c.filters.ts +++ b/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 { const method = 'filters.undo'; const client = sieveClient(method); - const change = redeemUndoToken(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(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); + }); } diff --git a/lib/plugin-sandbox/assistant/c.settings.ts b/lib/plugin-sandbox/assistant/c.settings.ts index 5fb4d85f9..09497b8e5 100644 --- a/lib/plugin-sandbox/assistant/c.settings.ts +++ b/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 { const method = 'app.undoSettings'; const client = sessionClient(method); - const previous = redeemUndoToken>(method, pluginId, undoAccount(client), 'settings', token); + return withUndoToken, 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): Promise { const restored: string[] = []; const refused: SettingsUndoResult['refused'] = {}; for (const [key, old] of Object.entries(previous)) { diff --git a/lib/plugin-sandbox/assistant/c.undo.ts b/lib/plugin-sandbox/assistant/c.undo.ts index 6a59d0d6e..dedf201f2 100644 --- a/lib/plugin-sandbox/assistant/c.undo.ts +++ b/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(); @@ -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(method: string, pluginId: string, accountId: string, kind: string, token: unknown): T { +export async function withUndoToken( + method: string, pluginId: string, accountId: string, kind: string, token: unknown, run: (data: T) => Promise, +): Promise { 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. */