fix(email): route unified/cross virtual ids through the fan-out in fetchEmails (#791)

fetchEmails sent a unified/cross view's client-only virtual mailbox id (__cross_all__ / __unified_*) straight to getEmails as `inMailbox`. The server rejects it (Stalwart: "filter.inMailbox = '__cross_all__'") and the foreground catch wiped the list to "no messages found" - notably after actions (mark read, delete, move) in the aggregated views. Mirror the loadMoreEmails / refreshCurrentMailbox guard: when the target id is virtual, fan out via fetchCrossViewEmails / fetchUnifiedEmails (search-aware) at page 0 instead of querying the virtual id. If the view state isn't resolved yet, stop loading without sending the id or wiping the list. Real folders are unaffected. +5 unit tests (email-store-fetch-unified): cross/unified fan-out, explicit virtual-id arg, unresolved-view no-op, and the real-folder path still using getEmails. tsc + eslint clean; all store suites green (389).
2026-11-01 16:12:02 +00:00 · 2026-08-13 21:27:24 +02:00
parent f2365e66cc
commit b63b227645
2 changed files with 205 additions and 0 deletions
@@ -0,0 +1,162 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+/**
+ * #791: fetchEmails is also called with a unified/cross view's VIRTUAL mailbox
+ * id (e.g. __cross_all__) - notably after actions in the aggregated views. The
+ * single-mailbox path sent that virtual id to getEmails as `inMailbox`; the
+ * server rejects it and the (foreground) catch wipes the list to "no messages".
+ * These tests pin that fetchEmails fans out via the unified loaders and never
+ * sends the virtual id, so the list stays intact.
+ */
+
+const { buildMock, fetchCrossViewMock, fetchUnifiedMock } = vi.hoisted(() => ({
+  buildMock: vi.fn(async () => [] as unknown[]),
+  fetchCrossViewMock: vi.fn(),
+  fetchUnifiedMock: vi.fn(),
+}));
+
+vi.mock('@/lib/unified-mailbox', async (importActual) => {
+  const actual = await importActual<typeof import('@/lib/unified-mailbox')>();
+  return {
+    ...actual,
+    buildUnifiedAccountClients: buildMock,
+    fetchCrossViewEmails: fetchCrossViewMock,
+    fetchUnifiedEmails: fetchUnifiedMock,
+  };
+});
+
+import { useEmailStore } from '../email-store';
+import { useSettingsStore } from '../settings-store';
+import { CROSS_ALL, UNIFIED_INBOX } from '@/lib/jmap/types';
+import type { Email } from '@/lib/jmap/types';
+import type { IJMAPClient } from '@/lib/jmap/client-interface';
+
+const makeEmail = (id: string, keywords: Record<string, boolean> = {}): Email =>
+  ({
+    id,
+    threadId: `t-${id}`,
+    mailboxIds: { inbox: true },
+    keywords,
+    from: [{ email: 'a@example.com' }],
+    to: [{ email: 'b@example.com' }],
+    subject: `mail ${id}`,
+    receivedAt: '2026-08-13T10:00:00Z',
+    preview: '',
+    hasAttachment: false,
+    size: 1,
+  }) as unknown as Email;
+
+// A client whose getEmails returns an empty page for the virtual id - the
+// pre-fix bug. If fetchEmails ever calls it in a unified view, the list wipes.
+const makeVirtualIdClient = () =>
+  ({
+    getEmails: vi.fn(async () => ({ emails: [], hasMore: false, total: 0 })),
+  }) as unknown as IJMAPClient & { getEmails: ReturnType<typeof vi.fn> };
+
+describe('fetchEmails in unified / cross-account views (#791)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    buildMock.mockResolvedValue([]);
+    useSettingsStore.setState({ emailsPerPage: 25 });
+    useEmailStore.setState({
+      emails: [],
+      totalEmails: 0,
+      searchQuery: '',
+      isUnifiedView: false,
+      unifiedRole: null,
+      crossView: null,
+    });
+  });
+
+  it('cross-account "All Mail" fans out and keeps the list instead of emptying it', async () => {
+    fetchCrossViewMock.mockResolvedValue({
+      emails: [makeEmail('a'), makeEmail('b', { $seen: true })],
+      hasMore: false,
+      total: 2,
+      errors: new Map(),
+    });
+    useEmailStore.setState({
+      selectedMailbox: CROSS_ALL,
+      isUnifiedView: true,
+      crossView: 'all',
+      emails: [makeEmail('a'), makeEmail('b')],
+      totalEmails: 2,
+    });
+
+    const client = makeVirtualIdClient();
+    await useEmailStore.getState().fetchEmails(client);
+
+    const emails = useEmailStore.getState().emails;
+    expect(emails.map((e) => e.id)).toEqual(['a', 'b']); // not emptied
+    expect(emails.find((e) => e.id === 'b')?.keywords?.$seen).toBe(true);
+    expect(fetchCrossViewMock).toHaveBeenCalledTimes(1);
+    expect(client.getEmails).not.toHaveBeenCalled(); // never sent __cross_all__
+  });
+
+  it('unified-role view fans out via the unified loader, not the virtual id', async () => {
+    fetchUnifiedMock.mockResolvedValue({
+      emails: [makeEmail('a'), makeEmail('b')],
+      hasMore: false,
+      total: 2,
+      errors: new Map(),
+    });
+    useEmailStore.setState({
+      selectedMailbox: UNIFIED_INBOX,
+      isUnifiedView: true,
+      unifiedRole: 'inbox',
+      crossView: null,
+      emails: [makeEmail('a')],
+      totalEmails: 1,
+    });
+
+    const client = makeVirtualIdClient();
+    await useEmailStore.getState().fetchEmails(client);
+
+    expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['a', 'b']);
+    expect(fetchUnifiedMock).toHaveBeenCalledTimes(1);
+    expect(client.getEmails).not.toHaveBeenCalled();
+  });
+
+  it('an explicit virtual mailboxId argument is also routed (not sent to getEmails)', async () => {
+    fetchCrossViewMock.mockResolvedValue({ emails: [makeEmail('a')], hasMore: false, total: 1, errors: new Map() });
+    useEmailStore.setState({ isUnifiedView: true, crossView: 'all', selectedMailbox: 'inbox' });
+
+    const client = makeVirtualIdClient();
+    await useEmailStore.getState().fetchEmails(client, CROSS_ALL);
+
+    expect(fetchCrossViewMock).toHaveBeenCalledTimes(1);
+    expect(client.getEmails).not.toHaveBeenCalled();
+  });
+
+  it('a virtual id with no resolved view leaves the list untouched (no query, no wipe)', async () => {
+    useEmailStore.setState({
+      selectedMailbox: CROSS_ALL,
+      isUnifiedView: true,
+      crossView: null,
+      unifiedRole: null,
+      emails: [makeEmail('a'), makeEmail('b')],
+      totalEmails: 2,
+    });
+
+    const client = makeVirtualIdClient();
+    await useEmailStore.getState().fetchEmails(client);
+
+    expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['a', 'b']); // not wiped
+    expect(fetchCrossViewMock).not.toHaveBeenCalled();
+    expect(fetchUnifiedMock).not.toHaveBeenCalled();
+    expect(client.getEmails).not.toHaveBeenCalled();
+  });
+
+  it('a real folder still uses the single-mailbox getEmails path', async () => {
+    const client = ({
+      getEmails: vi.fn(async () => ({ emails: [makeEmail('r')], hasMore: false, total: 1 })),
+    }) as unknown as IJMAPClient & { getEmails: ReturnType<typeof vi.fn> };
+    useEmailStore.setState({ selectedMailbox: 'inbox', isUnifiedView: false, crossView: null, unifiedRole: null });
+
+    await useEmailStore.getState().fetchEmails(client, 'inbox');
+
+    expect(client.getEmails).toHaveBeenCalledTimes(1);
+    expect(fetchCrossViewMock).not.toHaveBeenCalled();
+    expect(fetchUnifiedMock).not.toHaveBeenCalled();
+  });
+});
@@ -1160,6 +1160,49 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
        await get().fetchScheduledEmails(client);
        return;
      }
+
+      // Unified / cross-account views use client-only virtual mailbox ids
+      // (`__cross_*`, `__unified_*`) that no JMAP server accepts as `inMailbox`.
+      // Route them through the fan-out loaders (mirrors loadMoreEmails /
+      // refreshCurrentMailbox) instead of sending the virtual id to getEmails,
+      // which the server rejects and the catch below then empties the list (#791).
+      if (isCrossViewId(targetMailboxId) || isUnifiedMailboxId(targetMailboxId)) {
+        const { crossView, unifiedRole, searchQuery, searchFilters } = get();
+        // View state not resolved yet: don't send a virtual id and don't wipe
+        // the current list - just stop the loading state.
+        if (!crossView && !unifiedRole) {
+          set({ isLoading: false });
+          return;
+        }
+        const emailsPerPage = useSettingsStore.getState().emailsPerPage;
+        const includeGroup = useSettingsStore.getState().includeGroupInUnified;
+        const built = await buildUnifiedAccountClients({ includeGroup });
+        const hasFilters = !isFilterEmpty(searchFilters);
+        const result = crossView
+          ? (hasFilters
+              ? await advancedSearchCrossViewEmails(built, crossView, buildJMAPFilter(searchQuery, searchFilters, undefined), emailsPerPage, 0)
+              : searchQuery
+                ? await searchCrossViewEmails(built, crossView, searchQuery, emailsPerPage, 0)
+                : await fetchCrossViewEmails(built, crossView, emailsPerPage, 0))
+          : (hasFilters
+              ? await advancedSearchUnifiedEmails(built, unifiedRole!, (mb) => buildJMAPFilter(searchQuery, searchFilters, mb), emailsPerPage, 0)
+              : searchQuery
+                ? await searchUnifiedEmails(built, unifiedRole!, searchQuery, emailsPerPage, 0)
+                : await fetchUnifiedEmails(built, unifiedRole!, emailsPerPage, 0));
+        const enrichedEmails = await emailHooks.onEmailsFetched.transform(result.emails);
+        set({
+          emails: annotateScheduledEmails(enrichedEmails, get().scheduledSubmissionByEmailId),
+          hasMoreEmails: result.hasMore,
+          totalEmails: result.total,
+          threadEmailsCache: new Map(),
+          expandedThreadIds: new Set(),
+          isLoadingThread: null,
+          isLoading: false,
+          unifiedErrors: result.errors,
+        });
+        return;
+      }
+
      const effectiveClient = resolveActionClient(client);

      // Find the mailbox to get its accountId (for shared folder support)