fix: give plugins plain message data, let them add only real messages to search results and skip a draft save

2026-10-30 16:16:19 +00:00 · 2026-10-14 04:05:43 +02:00
parent c464ca2030
commit 0a98ffa711
11 changed files with 204 additions and 110 deletions
@@ -312,3 +312,43 @@ describe('onBeforeCompose prefill', () => {
    expect(screen.getByText('bob@example.com')).toBeInTheDocument();
  });
 });
+
+describe('onBeforeDraftAutoSave', () => {
+  function withClient(createDraft: ReturnType<typeof vi.fn>) {
+    useAuthStore.setState({
+      client: { createDraft, getEmail: vi.fn(), hasDelayedSend: () => false, getMaxDelayedSend: () => 0 } as never,
+    });
+  }
+
+  it('lets a plugin skip a save while typing', async () => {
+    const before = vi.fn().mockReturnValue(false);
+    emailHooks.onBeforeDraftAutoSave.register('test', before);
+    const createDraft = vi.fn().mockResolvedValue('draft-1');
+    withClient(createDraft);
+    try {
+      render(<EmailComposer initialData={DRAFT} />);
+      fireEvent.change(screen.getByTestId('composer-subject'), { target: { value: 'Lunch?' } });
+      await waitFor(() => expect(before).toHaveBeenCalled(), { timeout: 4000 });
+      await act(async () => {});
+
+      expect(createDraft).not.toHaveBeenCalled();
+    } finally {
+      useAuthStore.setState({ client: null });
+    }
+  }, 15_000);
+
+  it('saves the draft a plugin returns', async () => {
+    emailHooks.onBeforeDraftAutoSave.register('test', (draft: { subject: string }) => ({ ...draft, subject: `[x] ${draft.subject}` }));
+    const createDraft = vi.fn().mockResolvedValue('draft-1');
+    withClient(createDraft);
+    try {
+      render(<EmailComposer initialData={DRAFT} />);
+      fireEvent.change(screen.getByTestId('composer-subject'), { target: { value: 'Lunch?' } });
+      await waitFor(() => expect(createDraft).toHaveBeenCalledTimes(1), { timeout: 4000 });
+
+      expect(createDraft.mock.calls[0][1]).toBe('[x] Lunch?');
+    } finally {
+      useAuthStore.setState({ client: null });
+    }
+  }, 15_000);
+});
@@ -357,6 +357,9 @@ export function normalizePluginAttachment(raw: unknown): { name: string; type: s
  return { name, type, size: Math.floor(r.size), blobId: r.blobId };
 }

+/** How long a plugin may take over onBeforeDraftAutoSave before the save goes on without it. */
+const DRAFT_HOOK_TIMEOUT_MS = 5000;
+
 export function EmailComposer({
  onSend,
  onScheduledSendCreated,
@@ -2131,7 +2134,10 @@ export function EmailComposer({
  }, [previewAttachment, composerClient]);

  // Auto-save draft functionality
-  const saveDraftOnce = async (): Promise<string | null> => {
+  // `auto`: the debounced save while typing. Only that one may be skipped
+  // by a plugin; saves on close and before send always run, or the text
+  // would be lost.
+  const saveDraftOnce = async (auto = false): Promise<string | null> => {
    if (!client || !composerClient) return null;
    const versionAtStart = changeVersionRef.current;

@@ -2234,7 +2240,19 @@ export function EmailComposer({
        fromName,
        htmlBody: draftHtmlBody
      }
-      savedDraft = await emailHooks.onBeforeDraftAutoSave.transform(savedDraft);
+      if (emailHooks.onBeforeDraftAutoSave.size > 0) {
+        const outcome = await emailHooks.onBeforeDraftAutoSave.interceptWithValue(
+          savedDraft,
+          (result) => (result && typeof result === 'object' ? result as typeof savedDraft : undefined),
+          { timeoutMs: DRAFT_HOOK_TIMEOUT_MS },
+        );
+        if (!outcome.allowed && auto) {
+          // Skipped: leave lastSavedDataRef alone so the next save retries.
+          setSaveStatus('idle');
+          return draftIdRef.current;
+        }
+        if (outcome.allowed) savedDraft = outcome.value;
+      }

      // Use the JMAP client and raw identity id for the *owning* account
      // - falls back to active client for single-account / same-account
@@ -2321,13 +2339,13 @@ export function EmailComposer({
  // racing two `Email/set { destroy, create }` requests against the same
  // draftId, which left orphan drafts and (when EmailSubmission failed)
  // looked like "send didn't happen" (#303).
-  const saveDraft = (): Promise<string | null> => {
+  const saveDraft = (opts: { auto?: boolean } = {}): Promise<string | null> => {
    const previous = inflightSaveRef.current;
    const promise = (async (): Promise<string | null> => {
      if (previous) {
        try { await previous; } catch { /* prior failure already reported */ }
      }
-      return saveDraftOnce();
+      return saveDraftOnce(!!opts.auto);
    })();
    inflightSaveRef.current = promise;
    promise.finally(() => {
@@ -2372,7 +2390,7 @@ export function EmailComposer({
          .filter(a => a.blobId && !a.uploading && !a.error)
          .map(a => ({ name: a.name, type: a.type || 'application/octet-stream', size: a.size })),
      });
-      saveDraft();
+      saveDraft({ auto: true });
    }, 2000);

    // Cleanup on unmount
@@ -37,16 +37,14 @@ const sources = ['app', 'components', 'contexts', 'hooks', 'lib', 'stores'].flat

 /** A call site that fires the hook: `xHooks.onFoo.emit(` and friends. */
 function isFired(hookName: string, groupName: string): boolean {
-  const re = new RegExp(`\\b${groupName}\\.${hookName}\\s*\\.(emit|emitSync|emitTo|intercept|interceptOutcome|transform|takeOver)\\b`);
+  const re = new RegExp(`\\b${groupName}\\.${hookName}\\s*\\.(emit|emitSync|emitTo|intercept|interceptOutcome|interceptWithValue|transform|takeOver)\\b`);
  return re.test(sources);
 }

 // Groups whose call sites are still being wired in this branch. Each
 // commit that wires a group removes it here, and the test then insists every
 // hook in it has a call site.
-const NOT_YET_WIRED = new Set<HookGroup>([
-  'mail-list', 'mail-read', 'mail-compose', 'mail-send', 'mail-organize', 'mail-mailboxes', 'mail-search',
-  'mail-sync', 'settings', 'identities',
+const NOT_YET_WIRED = new Set<HookGroup>([ 'settings', 'identities',
  'filters', 'vacation', 'templates', 'security'
 ]);

@@ -3,7 +3,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { clearAllHooks, uiHooks, pluginErrorTracker } from '../plugin-hooks';
 import { interceptWithResult, acceptLinkRewrite, resolveExternalLink } from '../plugin-hook-result';

-const plainIntercept = (v: unknown) => uiHooks.onBeforeExternalLink.intercept(v);

 beforeEach(() => {
  clearAllHooks();
@@ -13,7 +12,7 @@ beforeEach(() => {
 describe('interceptWithResult', () => {
  it('lets the action through unchanged without handlers', async () => {
    const ctx = { href: 'https://a.example/' };
-    expect(await interceptWithResult(uiHooks.onBeforeExternalLink, ctx, acceptLinkRewrite, plainIntercept))
+    expect(await interceptWithResult(uiHooks.onBeforeExternalLink, ctx, acceptLinkRewrite))
      .toEqual({ allowed: true, value: ctx });
  });

@@ -21,7 +20,7 @@ describe('interceptWithResult', () => {
    const later = vi.fn();
    uiHooks.onBeforeExternalLink.register('a', () => false);
    uiHooks.onBeforeExternalLink.register('b', later);
-    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite, plainIntercept);
+    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite);
    expect(out).toEqual({ allowed: false });
    expect(later).not.toHaveBeenCalled();
  });
@@ -30,7 +29,7 @@ describe('interceptWithResult', () => {
    const second = vi.fn(() => undefined);
    uiHooks.onBeforeExternalLink.register('a', () => ({ href: 'https://proxy.example/?u=1' }));
    uiHooks.onBeforeExternalLink.register('b', second);
-    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/', emailId: 'e1' }, acceptLinkRewrite, plainIntercept);
+    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/', emailId: 'e1' }, acceptLinkRewrite);
    expect(out).toEqual({ allowed: true, value: { href: 'https://proxy.example/?u=1', emailId: 'e1' } });
    expect(second).toHaveBeenCalledWith({ href: 'https://proxy.example/?u=1', emailId: 'e1' });
  });
@@ -38,13 +37,13 @@ describe('interceptWithResult', () => {
  it('calls every handler once', async () => {
    const handler = vi.fn(() => ({ href: 'https://b.example/' }));
    uiHooks.onBeforeExternalLink.register('a', handler);
-    await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite, plainIntercept);
+    await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite);
    expect(handler).toHaveBeenCalledTimes(1);
  });

  it('skips a handler that throws (fails open)', async () => {
    uiHooks.onBeforeExternalLink.register('a', () => { throw new Error('boom'); });
-    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite, plainIntercept);
+    const out = await interceptWithResult(uiHooks.onBeforeExternalLink, { href: 'https://a.example/' }, acceptLinkRewrite);
    expect(out).toEqual({ allowed: true, value: { href: 'https://a.example/' } });
  });
 });
@@ -6,7 +6,6 @@

 import type { ComposeOptions } from './plugin-types';
 import { emailHooks } from './plugin-hooks';
-import { interceptWithResult } from './plugin-hook-result';
 import { buildReplyRecipients, type ReplySource } from './reply-recipients';
 import { buildForwardSubject, buildReplySubject } from './subject-prefix';
 import { formatRecipient } from './email-composer-utils';
@@ -62,7 +61,9 @@ const sameList = (a: string[], b: string[]) => a.length === b.length && a.every(
 * otherwise to the fields it changed (empty when none).
 */
 export async function runBeforeCompose(options: ComposeOptions): Promise<ComposePrefill | null> {
-  const outcome = await interceptWithResult(emailHooks.onBeforeCompose, options, acceptComposeOptions, (o) => emailHooks.onBeforeCompose.intercept(o));
+  const outcome = emailHooks.onBeforeCompose.size > 0
+    ? await emailHooks.onBeforeCompose.interceptWithValue(options, acceptComposeOptions)
+    : { allowed: true as const, value: options };
  if (!outcome.allowed) return null;
  const changed = outcome.value;
  const prefill: ComposePrefill = {};
@@ -65,7 +65,7 @@ export const HOOK_CATALOG = {
  onComposerOpen: H('mail-compose', 'observer', 'email:read', 'ComposerContext', 'after the composer opened'),
  onComposerClose: H('mail-compose', 'observer', 'email:read', '{ draftId, sent: boolean }', 'after the composer closed'),
  onDraftChange: H('mail-compose', 'observer', 'email:read', 'DraftView', 'while the user writes (debounced)'),
-  onBeforeDraftAutoSave: H('mail-compose', 'transform', 'email:send', 'AlmostSavedDraft', 'before a draft is saved; return false to skip this save'),
+  onBeforeDraftAutoSave: H('mail-compose', 'transform', 'email:send', 'AlmostSavedDraft', 'before a draft is saved; return a changed draft, or false to skip a save while typing (saves on close and before send always run)'),
  onDraftAutoSave: H('mail-compose', 'observer', 'email:read', '{ draftId, subject }', 'after a draft was saved'),
  onBeforeEditDraft: H('mail-compose', 'transform', 'email:send', 'Email', 'before a saved draft is opened in the composer'),
  onBuildQuoteHeader: H('mail-compose', 'transform', 'email:send', 'QuoteHeader, QuoteHeaderContext', 'when a reply or forward is set up'),
@@ -115,7 +115,7 @@ export const HOOK_CATALOG = {

  // ─── Mail: search ──────────────────────────────────────────
  onSearch: H('mail-search', 'transform', 'email:read', '{ query, filters: SearchFilters }', 'before a search runs; may change the query and filters'),
-  onSearchResults: H('mail-search', 'observer', 'email:read', '{ query, count }', 'after search results arrived'),
+  onSearchResults: H('mail-search', 'transform', 'email:read', 'string[], { query, filters, emailIds }', 'after search results arrived; return ids of further messages to add to them (at most 100)'),
  onProvideSearchResults: H('mail-search', 'transform', 'email:read', 'ExternalSearchResult[], { query, filters }', 'to add results from elsewhere to a search'),

  // ─── Mail: attachments ─────────────────────────────────────
@@ -1,83 +1,28 @@
 // "Before" hooks whose handlers may either stop an action (return false) or
 // change what it does (return a value), such as onBeforeExternalLink
-// (`{ href }`) and onBeforeCompose (a ComposeOptions to prefill).
-//
-// HookBus.intercept() only reports whether the action may go ahead and drops
-// every other return value; HookBus.transform() keeps return values but
-// ignores `false`. Running both would call every plugin twice (and show any
-// confirmation dialog twice), so this walks the handlers once and honours
-// both answers. A payload changed in place has no effect: plugin handlers run
-// in a sandboxed iframe and receive a copy.
+// (`{ href }`) and onBeforeCompose (a ComposeOptions to prefill). A payload
+// changed in place has no effect: plugin handlers run in a sandboxed iframe
+// and receive a copy.

-import { pluginErrorTracker, uiHooks, type HookBus } from './plugin-hooks';
+import { uiHooks, type HookBus } from './plugin-hooks';
 import type { ExternalLinkContext } from './plugin-types';
 import { isOpenableLinkHref } from './email-sanitization';

-// Same budget as HookBus.intercept: these hooks may wait on a dialog.
-const INTERCEPT_TIMEOUT_MS = 60_000;
-
-interface Entry {
-  pluginId: string;
-  handler: (...args: unknown[]) => unknown;
-}
-
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function handlersOf(bus: HookBus<any>): Entry[] | null {
-  const handlers = (bus as unknown as { handlers?: unknown }).handlers;
-  return Array.isArray(handlers) ? [...(handlers as Entry[])] : null;
-}
-
-function withTimeout<T>(value: T | Promise<T>, ms: number): Promise<T> {
-  if (!(value instanceof Promise)) return Promise.resolve(value);
-  return new Promise<T>((resolve, reject) => {
-    const timer = setTimeout(() => reject(new Error(`Hook timed out after ${ms}ms`)), ms);
-    value.then(
-      (v) => { clearTimeout(timer); resolve(v); },
-      (e) => { clearTimeout(timer); reject(e); },
-    );
-  });
-}
-
 export type InterceptResult<V> = { allowed: false } | { allowed: true; value: V };

 /**
- * Ask every handler of an intercept bus, in order. `false` stops the action;
- * any other return value goes through `accept`, which turns it into the next
- * value or returns undefined to ignore it. Like `intercept`, a handler that
- * throws or times out is skipped (fail open).
- *
- * `plainIntercept` is the bus's own `intercept`, used when its handlers can't
- * be walked (a test double): the veto still works, return values are lost.
+ * Ask every handler of an intercept bus once, in order. `false` stops the
+ * action; any other return value goes through `accept`. See
+ * HookBus.interceptWithValue.
 */
 export async function interceptWithResult<V>(
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  bus: HookBus<any>,
  value: V,
  accept: (result: unknown, current: V) => V | undefined,
-  plainIntercept: (value: V) => Promise<boolean>,
 ): Promise<InterceptResult<V>> {
-  // No listeners (or a test double without a size): nothing to ask.
  if (!(bus && bus.size > 0)) return { allowed: true, value };
-  const handlers = handlersOf(bus);
-  if (!handlers) {
-    return (await plainIntercept(value)) ? { allowed: true, value } : { allowed: false };
-  }
-  let current = value;
-  for (const { pluginId, handler } of handlers) {
-    if (pluginErrorTracker.isDisabled(pluginId)) continue;
-    let result: unknown;
-    try {
-      result = await withTimeout(handler(current), INTERCEPT_TIMEOUT_MS);
-    } catch (err) {
-      pluginErrorTracker.record(pluginId, err);
-      continue;
-    }
-    if (result === false) return { allowed: false };
-    if (result === undefined || result === null || result === true) continue;
-    const next = accept(result, current);
-    if (next !== undefined) current = next;
-  }
-  return { allowed: true, value: current };
+  return bus.interceptWithValue(value, accept);
 }

 /**
@@ -105,7 +50,9 @@ export function acceptLinkRewrite<T extends { href: string }>(result: unknown, c
 * when a plugin stopped it.
 */
 export async function resolveExternalLink(ctx: ExternalLinkContext): Promise<string | null> {
-  const outcome = await interceptWithResult(uiHooks.onBeforeExternalLink, ctx, acceptLinkRewrite, (c) => uiHooks.onBeforeExternalLink.intercept(c));
+  const outcome = uiHooks.onBeforeExternalLink.size > 0
+    ? await uiHooks.onBeforeExternalLink.interceptWithValue(ctx, acceptLinkRewrite)
+    : { allowed: true as const, value: ctx };
  if (!outcome.allowed || !isOpenableLinkHref(outcome.value.href)) return null;
  return outcome.value.href;
 }
@@ -251,6 +251,38 @@ export class HookBus<T extends (...args: any[]) => any> {
    return 'default';
  }

+  /**
+   * For "before" hooks whose handlers may stop an action (return `false`) or
+   * change what it does (return a value). Handlers run once each, in order,
+   * with the intercept budget; `accept` turns a return value into the next
+   * value, or returns undefined to ignore it. A handler that throws or runs
+   * out of time is skipped (fail open), and counts as a failure like any
+   * other hook error.
+   */
+  async interceptWithValue<V>(
+    initial: V,
+    accept: (result: unknown, current: V) => V | undefined,
+    opts: { timeoutMs?: number } = {},
+  ): Promise<{ allowed: false } | { allowed: true; value: V }> {
+    const budget = opts.timeoutMs ?? Math.max(this.timeoutMs, INTERCEPT_TIMEOUT_MS);
+    let value = initial;
+    for (const { pluginId, handler } of this.handlers) {
+      if (pluginErrorTracker.isDisabled(pluginId)) continue;
+      let result: unknown;
+      try {
+        result = await withTimeout((handler as (v: V) => unknown)(value), budget);
+      } catch (err) {
+        this.recordFailure(pluginId, err);
+        continue;
+      }
+      if (result === false) return { allowed: false };
+      if (result === undefined || result === null || result === true) continue;
+      const next = accept(result, value);
+      if (next !== undefined) value = next;
+    }
+    return { allowed: true, value };
+  }
+
  /** Fire handlers as transforms - each receives the output of the previous */
  async transform<V>(initial: V, ...rest: unknown[]): Promise<V> {
    let value = initial;
@@ -0,0 +1,32 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { useEmailStore } from '../email-store';
+import { clearAllHooks, emailHooks } from '@/lib/plugin-hooks';
+import type { Email } from '@/lib/jmap/types';
+
+const email = (id: string) => ({
+  id, threadId: `t-${id}`, mailboxIds: { inbox: true }, keywords: {}, subject: `Subject ${id}`,
+  from: [{ name: 'Bob', email: 'bob@example.org' }], to: [], receivedAt: '2026-10-01T00:00:00Z',
+  preview: 'p', size: 1, hasAttachment: false, blobId: 'raw-blob',
+} as unknown as Email);
+
+beforeEach(() => {
+  clearAllHooks();
+  useEmailStore.setState({ selectedEmail: null });
+});
+
+describe('message open and close hooks', () => {
+  it('hands plugins plain views, not the store object', () => {
+    const opened = vi.fn();
+    const closed = vi.fn();
+    emailHooks.onEmailOpen.register('t', opened);
+    emailHooks.onEmailClose.register('t', closed);
+
+    useEmailStore.getState().selectEmail(email('a'));
+    useEmailStore.getState().selectEmail(email('b'));
+
+    expect(opened).toHaveBeenCalledTimes(2);
+    expect(opened.mock.calls[0][0]).toMatchObject({ id: 'a', subject: 'Subject a', from: [{ name: 'Bob', email: 'bob@example.org' }] });
+    expect(opened.mock.calls[0][0]).not.toHaveProperty('blobId');
+    expect(closed).toHaveBeenCalledWith({ emailId: 'a' });
+  });
+});
@@ -78,3 +78,28 @@ describe('onSearch', () => {
    expect(filter).not.toContain('bob@x.test');
  });
 });
+
+describe('onSearchResults', () => {
+  it('adds the messages a plugin names, fetched from the server, and nothing else', async () => {
+    vi.mocked(client.searchEmails).mockResolvedValue({ emails: [{ id: 'm1' }], hasMore: false, total: 1 } as never);
+    vi.mocked(client.getSomeEmails).mockResolvedValue([{ id: 'm9' }] as never);
+    const handler = vi.fn().mockReturnValue(['m9', 'm1', 42]);
+    emailHooks.onSearchResults.register('test', handler);
+
+    await useEmailStore.getState().searchEmails(client, 'invoice');
+
+    expect(handler).toHaveBeenCalledWith([], expect.objectContaining({ query: 'invoice', emailIds: ['m1'] }));
+    expect(client.getSomeEmails).toHaveBeenCalledWith(['m9'], undefined);
+    expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['m1', 'm9']);
+  });
+
+  it('ignores anything that is not a list of ids', async () => {
+    vi.mocked(client.searchEmails).mockResolvedValue({ emails: [{ id: 'm1' }], hasMore: false, total: 1 } as never);
+    emailHooks.onSearchResults.register('test', () => ({ result: { emails: [{ id: 'fake', subject: 'Pay now' }] } }));
+
+    await useEmailStore.getState().searchEmails(client, 'invoice');
+
+    expect(client.getSomeEmails).not.toHaveBeenCalled();
+    expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['m1']);
+  });
+});
@@ -8,7 +8,7 @@ import { orderKeywords, placeHeldRows, type HeldKeywords, type SortLevel } from
 import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty } from "@/lib/jmap/search-utils";
 import { emailHooks } from "@/lib/plugin-hooks";
 import { allowEmailMove, emitEmailMove, idsFromSelectionKeys, keywordChangeLists, mailboxToView, quotaToView } from "@/lib/plugin-mail-hooks";
-import { emailSummary } from "@/lib/plugin-projection";
+import { emailSummary, emailToReadView } from "@/lib/plugin-projection";
 import { resolveThreadRoute } from "@/lib/thread-routing";
 import { emailKeyFor, threadKeyFor, threadIdFromKey, KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils";
 import type { ExternalSearchResult } from "@/lib/plugin-types";
@@ -1944,6 +1944,18 @@ function mergeRetainedRows(previous: Email[], merged: Email[], retainedIds: Set<
  return out;
 }

+/** Most message ids a plugin may add to one search through onSearchResults. */
+const MAX_PLUGIN_SEARCH_IDS = 100;
+
+async function pluginSearchResultIds(query: string, filters: unknown, emails: Email[]): Promise<string[]> {
+  if (emailHooks.onSearchResults.size === 0) return [];
+  const shown = new Set(emails.map((e) => e.id));
+  const ids = await emailHooks.onSearchResults.transform([] as string[], { query, filters, emailIds: [...shown] });
+  if (!Array.isArray(ids)) return [];
+  return [...new Set(ids.filter((id): id is string => typeof id === 'string' && id.length > 0 && id.length < 256 && !shown.has(id)))]
+    .slice(0, MAX_PLUGIN_SEARCH_IDS);
+}
+
 export const useEmailStore = create<EmailStore>((set, get) => ({
  emails: [],
  mailboxes: [],
@@ -2083,11 +2095,13 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
  selectEmail: (email) => {
    const prev = get().selectedEmail;
    set({ selectedEmail: email, lastSelectedEmailKey: email ? emailKeyFor(email) : get().lastSelectedEmailKey });
-    if (prev && (!email || email.id !== prev.id)) {
-      emailHooks.onEmailClose.emitSync(prev);
+    // Plain data only: handlers in a sandbox get a copy, and the raw store
+    // object carries fields no plugin permission covers.
+    if (prev && (!email || email.id !== prev.id) && emailHooks.onEmailClose.size > 0) {
+      emailHooks.onEmailClose.emitSync({ emailId: prev.id });
    }
-    if (email && (!prev || email.id !== prev.id)) {
-      emailHooks.onEmailOpen.emitSync(email);
+    if (email && (!prev || email.id !== prev.id) && emailHooks.onEmailOpen.size > 0) {
+      emailHooks.onEmailOpen.emitSync(emailToReadView(email));
    }
  },
  holdListRow: (rowKey) => {
@@ -3650,17 +3664,11 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
        result = await resolveActionClient(client).searchEmails(query, jmapMailboxId, accountId, emailsPerPage, 0);
      }

-      const hookEdit = await emailHooks.onSearchResults.transform({
-        newEmailIds: [] as string[],
-        result: result,
-        query: query,
-        filters: searchFilters
-      });
-
-      result = hookEdit.result;
-      if (hookEdit.newEmailIds.length > 0) {
-        // in unified, accountId will be undefined and we will use the default.
-        const newEmails = await resolveActionClient(client).getSomeEmails(hookEdit.newEmailIds, accountId);
+      // Plugins may add messages to the results by id; the messages are
+      // fetched from the server, so a plugin can't put made-up rows in the list.
+      const extraIds = await pluginSearchResultIds(query, searchFilters, result.emails);
+      if (extraIds.length > 0) {
+        const newEmails = await resolveActionClient(client).getSomeEmails(extraIds, accountId);
        result.emails.push(...newEmails);
        result.total += newEmails.length;
      }
@@ -3776,17 +3784,11 @@ export const useEmailStore = create<EmailStore>((set, get) => ({

      if (controller.signal.aborted) return;

-      const hookEdit = await emailHooks.onSearchResults.transform({ 
-        newEmailIds: [] as string[], 
-        result: result, 
-        query: searchQuery, 
-        filters: searchFilters 
-      });
-
-      result = hookEdit.result;
-
-      if (hookEdit.newEmailIds.length > 0) {
-        const newEmails = await resolveActionClient(client).getSomeEmails(hookEdit.newEmailIds, accountId);
+      // Plugins may add messages to the results by id; the messages are
+      // fetched from the server, so a plugin can't put made-up rows in the list.
+      const extraIds = await pluginSearchResultIds(searchQuery, searchFilters, result.emails);
+      if (extraIds.length > 0) {
+        const newEmails = await resolveActionClient(client).getSomeEmails(extraIds, accountId);
        result.emails.push(...newEmails);
        result.total += newEmails.length;
      }