fix: load the folder list once, and right away, when signing in

Signing in sent two full `Mailbox/get`s before the inbox loaded, and
the first only went out once the main screen had mounted and painted.
There is no deliberate delay behind the gap: the mail screen's mount
effect started the folder load, and the push setup read the same
folders again with a `Mailbox/get` of its own (plus the shared
accounts') just to find the Junk folders for its filter. The latter
also happened on every start with push notifications on.

- Sign-in (password and OAuth/pairing) starts the folder load as soon
  as the session is up, like a restored session already did.
- The mail screen's mount now joins a load on its way
  (`ensureMailboxes`) instead of queueing another behind it.
- The push filter takes the folders from the mail store (joining its
  load) and only fetches them itself when the store has none for the
  account.

src/lib/__tests__/push-config-mailboxes.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+// The push filter needs the Junk folders. It used to send its own full
+// Mailbox/get (and the shared accounts') on every sign-in and start, next to
+// the mail store's identical load (PF7).
+
+vi.mock('../../api/jmap-client', () => ({
+  jmapClient: {
+    username: 'user@example.com',
+    serverUrl: 'https://mail.example.com',
+    accountId: 'jmap-primary',
+    currentSession: { capabilities: {} },
+  },
+}));
+
+vi.mock('../../api/email', () => ({
+  getMailboxes: vi.fn(async () => [{ id: 'junk-own', role: 'junk', accountId: 'jmap-primary' }]),
+  getSharedMailboxes: vi.fn(async () => []),
+}));
+
+import { getMailboxes, getSharedMailboxes } from '../../api/email';
+import { buildEmailPushConfig } from '../push-notifications';
+import { provideLoadedMailboxes } from '../mailbox-source';
+import { generateAccountId } from '../account-utils';
+
+const ACCOUNT_ID = generateAccountId('user@example.com', 'https://mail.example.com');
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  provideLoadedMailboxes(null);
+});
+
+describe('push filter folders', () => {
+  it('takes the Junk folders from the mail store instead of fetching them', async () => {
+    const provider = vi.fn(async () => [
+      { id: 'junk', role: 'junk', accountId: 'jmap-primary' },
+      { id: 'team:j', originalId: 'j', role: 'junk', accountId: 'team', isShared: true },
+    ] as never);
+    provideLoadedMailboxes(provider);
+
+    const config = await buildEmailPushConfig();
+
+    expect(provider).toHaveBeenCalledWith(ACCOUNT_ID);
+    expect(getMailboxes).not.toHaveBeenCalled();
+    expect(getSharedMailboxes).not.toHaveBeenCalled();
+    expect(config['jmap-primary'].filter).toEqual({
+      operator: 'AND',
+      conditions: [{ notKeyword: '$junk' }, { inMailboxOtherThan: ['junk'] }],
+    });
+    expect(config.team.filter).toEqual({
+      operator: 'AND',
+      conditions: [{ notKeyword: '$junk' }, { inMailboxOtherThan: ['j'] }],
+    });
+  });
+
+  it('fetches them itself when the store has none for this account', async () => {
+    provideLoadedMailboxes(async () => null);
+
+    const config = await buildEmailPushConfig();
+
+    expect(getMailboxes).toHaveBeenCalledTimes(1);
+    expect(config['jmap-primary'].filter).toEqual({
+      operator: 'AND',
+      conditions: [{ notKeyword: '$junk' }, { inMailboxOtherThan: ['junk-own'] }],
+    });
+  });
+});

src/lib/mailbox-source.ts

@@ -0,0 +1,29 @@
+// The folder list the mail store holds (or is loading) for the signed-in
+// account, for code below the stores that needs it too: the push filter's
+// Junk folders. It used to send a full `Mailbox/get` of its own right next to
+// the store's on every sign-in and start. The mail store registers the
+// provider, so nothing here imports it.
+
+import type { Mailbox } from '../api/types';
+
+type Provider = (accountId: string) => Promise<Mailbox[] | null>;
+
+let provider: Provider | null = null;
+
+export function provideLoadedMailboxes(next: Provider | null): void {
+  provider = next;
+}
+
+/**
+ * The own and shared folders of `accountId` (the account registry id) once
+ * the store has them, joining a sync that is on its way. Null when the store
+ * serves another account or could not load them.
+ */
+export async function loadedMailboxes(accountId: string): Promise<Mailbox[] | null> {
+  if (!provider) return null;
+  try {
+    return await provider(accountId);
+  } catch {
+    return null;
+  }
+}

src/lib/push-notifications.ts

@@ -8,6 +8,7 @@ import {
   verifyPushSubscription,
 } from '../api/push';
 import { getMailboxes, getSharedMailboxes } from '../api/email';
+import { loadedMailboxes } from './mailbox-source';
 import { jmapClient } from '../api/jmap-client';
 import { JMAPMethodError } from '../api/jmap-result';
 import { CAPABILITIES } from '../api/types';
@@ -259,9 +260,16 @@ export function serverSupportsEmailPush(): boolean {
 export async function buildEmailPushConfig(): Promise<Record<string, EmailPushConfig>> {
   const primary = jmapClient.accountId;
   const junkByAccount = new Map<string, string[]>([[primary, []]]);
-  const own = await getMailboxes().catch(() => [] as Mailbox[]);
-  const shared = await getSharedMailboxes().catch(() => [] as Mailbox[]);
-  for (const m of [...own, ...shared]) {
+  // The mail store loads the same folders at the same moment (sign-in,
+  // start): reuse its list rather than a second Mailbox/get of our own.
+  const username = jmapClient.username;
+  const serverUrl = jmapClient.serverUrl;
+  const loaded = username && serverUrl ? await loadedMailboxes(generateAccountId(username, serverUrl)) : null;
+  const all = loaded ?? [
+    ...(await getMailboxes().catch(() => [] as Mailbox[])),
+    ...(await getSharedMailboxes().catch(() => [] as Mailbox[])),
+  ];
+  for (const m of all) {
     const accountId = m.accountId || primary;
     const junk = junkByAccount.get(accountId) ?? [];
     // Shared-account mailboxes carry a client-side "<account>:<id>" id;

src/screens/EmailListScreen.tsx

@@ -331,6 +331,7 @@ export default function EmailListScreen({ onEmailPress, onComposePress }: EmailL
   const filters = useEmailStore((s) => s.filters);
   const accountErrors = useEmailStore((s) => s.accountErrors);
   const fetchMailboxes = useEmailStore((s) => s.fetchMailboxes);
+  const ensureMailboxes = useEmailStore((s) => s.ensureMailboxes);
   const selectMailbox = useEmailStore((s) => s.selectMailbox);
   const loadMoreEmails = useEmailStore((s) => s.loadMoreEmails);
   const refreshEmails = useEmailStore((s) => s.refreshEmails);
@@ -937,12 +938,13 @@ export default function EmailListScreen({ onEmailPress, onComposePress }: EmailL
     );
   };
 
-  // Load mailboxes and select inbox on mount
+  // Load mailboxes and select inbox on mount (joining the load sign-in
+  // started rather than queueing another)
   React.useEffect(() => {
     if (mailboxes.length === 0) {
-      void fetchMailboxes();
+      void ensureMailboxes();
     }
-  }, [fetchMailboxes, mailboxes.length]);
+  }, [ensureMailboxes, mailboxes.length]);
 
   React.useEffect(() => {
     if (mailboxes.length > 0 && !currentMailboxId) {

src/stores/__tests__/auth-store-sign-in.test.ts

@@ -1,13 +1,15 @@
 import { describe, it, expect, vi } from 'vitest';
 
-// The display-name sync on every sign-in and start read the Stalwart
-// principal with x:Account/get, which is refused to everyone but admins
-// (PF6). The full name lives in x:AccountSettings, which every user reads.
+// What a sign-in sends (PF6, PF7): the display-name sync read the Stalwart
+// principal with x:Account/get, which is refused to everyone but admins (the
+// full name lives in x:AccountSettings, which every user reads), and the
+// folder list only started loading once the mail screen had mounted.
 
 const request = vi.fn(async (calls: Array<[string, Record<string, unknown>, string]>) => ({
   methodResponses: calls.map(([name, , id]) => {
     if (name === 'Identity/get') return [name, { list: [{ id: 'i1', name: 'Old Name', email: 'user@example.com' }] }, id];
     if (name === 'x:AccountSettings/get') return [name, { list: [{ description: 'Ada Lovelace' }] }, id];
+    if (name === 'Mailbox/get') return [name, { list: [{ id: 'mb-in', name: 'Inbox', role: 'inbox' }], state: 'mbs-1' }, id];
     return ['error', { type: 'forbidden' }, id];
   }),
 }));
@@ -20,6 +22,8 @@ vi.mock('../../api/jmap-client', () => ({
     onAuthFailure: vi.fn(() => () => undefined),
     onTokenRefresh: vi.fn(() => () => undefined),
     hasAccountCapability: vi.fn(() => true),
+    getAccountName: () => undefined,
+    getSharedMailAccounts: () => [],
     request: (calls: Array<[string, Record<string, unknown>, string]>) => request(calls),
     accountId: 'acc-1',
     isConnected: true,
@@ -38,6 +42,7 @@ vi.mock('../../lib/push-notifications', () => ({
 
 import { useAuthStore } from '../auth-store';
 import { useAccountStore } from '../account-store';
+import { useEmailStore } from '../email-store';
 
 describe('display name sync on sign-in', () => {
   it('reads the full name without asking for the principal', async () => {
@@ -50,4 +55,19 @@ describe('display name sync on sign-in', () => {
     expect(methods).toContain('x:AccountSettings/get');
     expect(methods).not.toContain('x:Account/get');
   });
+
+  it('starts on the folder list at once, and the mail screen joins that load', async () => {
+    request.mockClear();
+    useEmailStore.getState().clearAllAccounts();
+    await useAuthStore.getState().login('https://mail.example.com', 'user@example.com', 'pass');
+    const mailboxGets = () => request.mock.calls.filter(([calls]) => calls.some(([name]) => name.startsWith('Mailbox/')));
+    expect(mailboxGets()).toHaveLength(1);
+
+    // The mail screen mounts while that load is still on its way.
+    await useEmailStore.getState().ensureMailboxes();
+    expect(mailboxGets()).toHaveLength(1);
+    expect(useEmailStore.getState().mailboxes.map((m) => m.id)).toEqual(['mb-in']);
+    await useEmailStore.getState().ensureMailboxes();
+    expect(mailboxGets()).toHaveLength(1);
+  });
 });

src/stores/__tests__/email-store-own-writes.test.ts

@@ -85,6 +85,7 @@ import * as emailApi from '../../api/email';
 import { beginOwnWrite, recordOwnEmailWrites, resetOwnWrites } from '../../api/own-writes';
 import type { JMAPMethodCall } from '../../api/types';
 import { useEmailStore } from '../email-store';
+import { loadedMailboxes } from '../../lib/mailbox-source';
 
 const SERVER = 'https://mail.example.com';
 const mock = (fn: unknown) => fn as ReturnType<typeof vi.fn>;
@@ -291,3 +292,32 @@ describe('folder pushes', () => {
     expect(emailApi.getSharedMailboxes).toHaveBeenCalledTimes(1);
   });
 });
+
+describe('the folder list for the push filter', () => {
+  const ACCOUNT = generateAccountId('test@example.com', SERVER);
+
+  it('joins the load on its way instead of fetching again', async () => {
+    useEmailStore.setState({ activeAccountId: ACCOUNT, mailboxes: [], mailboxState: undefined });
+    mock(emailApi.getMailboxesWithState).mockResolvedValue({ list: [inboxFolder(0)], state: 'mbs-1' });
+    const loading = useEmailStore.getState().fetchMailboxes();
+
+    const list = await loadedMailboxes(ACCOUNT);
+    await loading;
+
+    expect(list?.map((m) => m.id)).toEqual(['mb-1']);
+    expect(emailApi.getMailboxesWithState).toHaveBeenCalledTimes(1);
+    expect(emailApi.getMailboxChanges).not.toHaveBeenCalled();
+  });
+
+  it('hands out a list it already holds without a request', async () => {
+    openInbox([row('e1')]);
+    expect((await loadedMailboxes(ACCOUNT))?.map((m) => m.id)).toEqual(['mb-1']);
+    expect(emailApi.getMailboxesWithState).not.toHaveBeenCalled();
+    expect(emailApi.getMailboxChanges).not.toHaveBeenCalled();
+  });
+
+  it('has nothing for another account', async () => {
+    openInbox([row('e1')]);
+    expect(await loadedMailboxes('someone-else@mail.example.com')).toBeNull();
+  });
+});

src/stores/auth-store.ts

@@ -224,6 +224,9 @@ async function completeOAuthHandoff(
   useEmailStore.getState().setActiveAccount(accountId);
 
   applyConnectedState(set, session, result.serverUrl.replace(/\/+$/, ''), username, accountId);
+  // Start on the folder list now rather than once the mail screen has
+  // mounted; the screen joins this load.
+  void useEmailStore.getState().fetchMailboxes();
   void syncAccountDisplayName(accountId);
 }
 
@@ -299,6 +302,9 @@ export const useAuthStore = create<AuthState>((set, get) => ({
 
       applyConnectedState(set, session, serverUrl.replace(/\/+$/, ''), username, accountId);
       set({ pendingTotpLogin: null });
+      // Start on the folder list now rather than once the mail screen has
+      // mounted; the screen joins this load.
+      void useEmailStore.getState().fetchMailboxes();
       void syncAccountDisplayName(accountId);
     } catch (err) {
       if (err instanceof Error && err.name === 'TotpRequiredError') {

src/stores/email-store.ts

@@ -34,6 +34,7 @@ import {
   unprefixMailboxId,
 } from '../api/email';
 import { applyOwnWritesToList, ownEmailWritesBetween, whenOwnWritesSettled } from '../api/own-writes';
+import { provideLoadedMailboxes } from '../lib/mailbox-source';
 import { useNetworkStore } from './network-store';
 import { JMAPMethodError } from '../api/jmap-result';
 import {
@@ -372,6 +373,12 @@ export interface EmailState {
   removeAccount: (accountId: string) => void;
   clearAllAccounts: () => void;
   fetchMailboxes: () => Promise<void>;
+  /**
+   * Load the folder list unless it is loaded or already loading: for
+   * mount-time callers, which would otherwise queue a second sync behind the
+   * one sign-in started.
+   */
+  ensureMailboxes: () => Promise<void>;
   selectMailbox: (mailboxId: string) => Promise<void>;
   loadMoreEmails: () => Promise<void>;
   refreshEmails: () => Promise<void>;
@@ -931,6 +938,13 @@ export const useEmailStore = create<EmailState>()(
     return syncMailboxes(activeAccountId!, { own: true, shared: true });
   },
 
+  ensureMailboxes: () => {
+    const { mailboxes, activeAccountId } = get();
+    if (mailboxes.length > 0 || !activeAccountId) return Promise.resolve();
+    const running = mailboxSyncsRunning(activeAccountId);
+    return running ?? get().fetchMailboxes();
+  },
+
   selectMailbox: async (mailboxId) => {
     const state = get();
     // Tuck the previously-visible mailbox into its snapshot so a return-trip
@@ -2096,6 +2110,23 @@ export const useEmailStore = create<EmailState>()(
   ),
 );
 
+// The folder syncs of an account on their way, joined (not queued behind).
+function mailboxSyncsRunning(activeAccountId: string): Promise<void> | null {
+  const running = [`${activeAccountId}:mailboxes`, `${activeAccountId}:shared-mailboxes`]
+    .map((key) => inflightRefresh.get(key))
+    .filter((p): p is Promise<void> => !!p);
+  return running.length > 0 ? Promise.all(running).then(() => undefined) : null;
+}
+
+// The push filter needs the Junk folders of every account: hand it this list
+// instead of letting it fetch its own (see lib/mailbox-source).
+provideLoadedMailboxes(async (accountId) => {
+  if (useEmailStore.getState().activeAccountId !== accountId) return null;
+  await (mailboxSyncsRunning(accountId) ?? useEmailStore.getState().ensureMailboxes());
+  const { mailboxes, activeAccountId } = useEmailStore.getState();
+  return activeAccountId === accountId && mailboxes.length > 0 ? mailboxes : null;
+});
+
 // Copy-then-delete across accounts (webmail `crossAccountMoveEmails`, 1.7.2):
 // download each message's blob from the source account, upload it to the
 // target account, Email/import it into the target folder with its keywords,