Merge pull request #1131 from lucamzanon/fix/deep-link-late-login

fix: let a notification link wait for a login that is still reconnecting

components/mail/mail-app.tsx

@@ -26,7 +26,7 @@ import { ProtocolAccountPicker } from "@/components/protocol/protocol-account-pi
 import { ThreadConversationView } from "@/components/email/thread-conversation-view";
 import { MobileHeader } from "@/components/layout/mobile-header";
 import { ThreadGroup, Email, Mailbox, isUnifiedMailboxId, UNIFIED_ROLE_BY_ID, CROSS_VIEW_BY_ID, isCrossViewId } from "@/lib/jmap/types";
-import { useAccountStore } from "@/stores/account-store";
+import { useAccountStore, waitForConnectedAccount } from "@/stores/account-store";
 import { usePolicyStore } from "@/stores/policy-store";
 import type { UnifiedAccountClient } from "@/lib/unified-mailbox";
 import { connectedAccountsGrew } from "@/lib/unified-mailbox";
@@ -134,6 +134,30 @@ export interface MailAppProps {
   linkSegments?: string[];
 }
 
+
+/**
+ * The folder list once it has been reloaded after an account switch. Until
+ * then the store holds the previous mailbox's folders, or none, and resolving
+ * a folder link against them reports a folder that is merely not loaded yet
+ * as gone - or, worse, finds the previous mailbox's folder of that name.
+ */
+function freshMailboxes(previous: Mailbox[], timeoutMs = 10_000): Promise<Mailbox[]> {
+  const ready = (mailboxes: Mailbox[]) => mailboxes !== previous && mailboxes.length > 0;
+  const now = useEmailStore.getState().mailboxes;
+  if (ready(now)) return Promise.resolve(now);
+  return new Promise((resolve) => {
+    const finish = (mailboxes: Mailbox[]) => {
+      clearTimeout(timer);
+      unsubscribe();
+      resolve(mailboxes);
+    };
+    const timer = setTimeout(() => finish(useEmailStore.getState().mailboxes), timeoutMs);
+    const unsubscribe = useEmailStore.subscribe((state) => {
+      if (ready(state.mailboxes)) finish(state.mailboxes);
+    });
+  });
+}
+
 export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
   // Static Lite build: the route params are empty, read the link from the URL.
   const linkSegments = useLiteLinkSegments('mail', routeSegments);
@@ -1367,6 +1391,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
   // this, the URL is an output of the view (see buildMailUrl), not an input.
   const deepLinkHandledRef = useRef(false);
   const applyMailDeepLink = async (link: MailDeepLink, opts?: { onLoad?: boolean }) => {
+    // The folder list before an account switch, if one happened: until the
+    // new mailbox's folders arrive the store still holds these (or nothing).
+    let switchedFrom: Mailbox[] | null = null;
     // A permalink can name the account it belongs to. Ids are only meaningful
     // within their account, so switch first - but only to a login that is
     // actually connected; we can't authenticate on someone's behalf. A push
@@ -1376,13 +1403,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
       : undefined);
     const switchesAccount = !!linkAccountId && linkAccountId !== useAuthStore.getState().activeAccountId;
     if (switchesAccount) {
-      const target = useAccountStore.getState().accounts.find(
-        (a) => a.id === linkAccountId && a.isConnected,
-      );
-      if (!target) {
+      // The login may still be reconnecting - a tapped notification reopens
+      // the app and the logins come back one at a time - so wait for it
+      // rather than calling a late mailbox unavailable.
+      if (!(await waitForConnectedAccount(linkAccountId))) {
         toast.error(t('deep_link.account_unavailable'));
         return;
       }
+      switchedFrom = useEmailStore.getState().mailboxes;
       await switchAccount(linkAccountId);
     }
 
@@ -1390,8 +1418,11 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
     if (!activeClient) return;
 
     if (link.kind === 'folder') {
+      const mailboxes = switchedFrom
+        ? await freshMailboxes(switchedFrom)
+        : useEmailStore.getState().mailboxes;
       const state = useEmailStore.getState();
-      const mailboxId = resolveFolderRef(link.ref, state.mailboxes);
+      const mailboxId = resolveFolderRef(link.ref, mailboxes);
       if (!mailboxId) {
         toast.error(t('deep_link.folder_not_found'));
         return;

stores/__tests__/account-store-wait-connected.test.ts

@@ -0,0 +1,69 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { useAccountStore, waitForConnectedAccount } from '../account-store';
+
+/**
+ * A tapped notification reopens the app and names a mailbox; the logins come
+ * back one at a time, so that mailbox is often not connected yet when the
+ * link is read. The wait must tell "late" from "never".
+ */
+function seed(accounts: Array<{ id: string; isConnected: boolean; hasError?: boolean }>) {
+  useAccountStore.setState({
+    accounts: accounts.map((a) => ({
+      id: a.id,
+      cookieSlot: 0,
+      username: a.id,
+      serverUrl: 'https://mail.example.org',
+      displayName: a.id,
+      email: a.id,
+      avatarColor: '#123456',
+      rememberMe: true,
+      isConnected: a.isConnected,
+      hasError: a.hasError ?? false,
+      isDefault: false,
+      lastLoginAt: 0,
+      label: a.id,
+      authMode: 'basic',
+    })) as never,
+  });
+}
+
+const connect = (id: string, patch: Record<string, unknown>) =>
+  useAccountStore.setState((s) => ({
+    accounts: s.accounts.map((a) => (a.id === id ? { ...a, ...patch } : a)),
+  }));
+
+describe('waitForConnectedAccount', () => {
+  beforeEach(() => vi.useFakeTimers());
+  afterEach(() => vi.useRealTimers());
+
+  it('answers at once for a login already connected', async () => {
+    seed([{ id: 'a', isConnected: true }]);
+    await expect(waitForConnectedAccount('a')).resolves.toBe(true);
+  });
+
+  it('waits for a login that is still reconnecting', async () => {
+    seed([{ id: 'a', isConnected: true }, { id: 'late', isConnected: false }]);
+    const waiting = waitForConnectedAccount('late');
+    connect('late', { isConnected: true });
+    await expect(waiting).resolves.toBe(true);
+  });
+
+  it('gives up on a login this browser does not have', async () => {
+    seed([{ id: 'a', isConnected: true }]);
+    await expect(waitForConnectedAccount('stranger')).resolves.toBe(false);
+  });
+
+  it('gives up when the login fails to reconnect', async () => {
+    seed([{ id: 'broken', isConnected: false }]);
+    const waiting = waitForConnectedAccount('broken');
+    connect('broken', { hasError: true });
+    await expect(waiting).resolves.toBe(false);
+  });
+
+  it('gives up after the timeout rather than waiting forever', async () => {
+    seed([{ id: 'stuck', isConnected: false }]);
+    const waiting = waitForConnectedAccount('stuck', 5_000);
+    vi.advanceTimersByTime(5_001);
+    await expect(waiting).resolves.toBe(false);
+  });
+});

stores/account-store.ts

@@ -251,3 +251,38 @@ export const useAccountStore = create<AccountState>()(
     }
   )
 );
+
+/**
+ * Resolves once the login `accountId` is connected, or with `false` when it
+ * never will be: not one of this browser's logins, its restore failed, or
+ * `timeoutMs` went by.
+ *
+ * A link can arrive while the browser is still reconnecting its logins - a
+ * notification tapped with the app closed does exactly that - and they come
+ * back one at a time, so the mailbox a link names is often not connected yet
+ * when the link is read. Treating "not yet" as "unavailable" sent the user to
+ * the wrong mailbox with an error; this waits for the login instead.
+ */
+export function waitForConnectedAccount(accountId: string, timeoutMs = 20_000): Promise<boolean> {
+  const settled = (account: AccountEntry | undefined): boolean | null => {
+    if (!account) return false;
+    if (account.isConnected) return true;
+    if (account.hasError) return false;
+    return null;
+  };
+  const now = settled(useAccountStore.getState().accounts.find((a) => a.id === accountId));
+  if (now !== null) return Promise.resolve(now);
+
+  return new Promise((resolve) => {
+    const finish = (value: boolean) => {
+      clearTimeout(timer);
+      unsubscribe();
+      resolve(value);
+    };
+    const timer = setTimeout(() => finish(false), timeoutMs);
+    const unsubscribe = useAccountStore.subscribe((state) => {
+      const result = settled(state.accounts.find((a) => a.id === accountId));
+      if (result !== null) finish(result);
+    });
+  });
+}