From 770fb2c84a0ca32638ccc9b95271a5d70f20ffed Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:34:38 +0200 Subject: [PATCH] fix: resolve trash and archive in a background login's own folders #1132 The folder-list cache from #1132 published a reused list only when it was freshly fetched. When the Pro folder tree had loaded a login first, the unified build reused that list and never stored it under the login's JMAP id, so trash and archive for that login's mail were resolved against the active login's folders until it pushed a change. A reused list now fills any key nothing has published yet, without overwriting a live one. The Pro folder tree also marked a login as loaded before its first fetch finished and never retried a failure, which left that login's tree empty until an account switch. A failed load is now retried on the next run, as the identities hook already does. --- .../use-pro-multi-account-mailboxes.test.tsx | 12 ++++++++++ hooks/use-pro-multi-account-mailboxes.ts | 5 ++++- .../email-store-unified-mailbox-cache.test.ts | 12 +++++++++- stores/email-store.ts | 22 +++++++++++-------- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/hooks/__tests__/use-pro-multi-account-mailboxes.test.tsx b/hooks/__tests__/use-pro-multi-account-mailboxes.test.tsx index 9201eb17e..5b6d9c669 100644 --- a/hooks/__tests__/use-pro-multi-account-mailboxes.test.tsx +++ b/hooks/__tests__/use-pro-multi-account-mailboxes.test.tsx @@ -71,6 +71,18 @@ describe('useProMultiAccountMailboxes', () => { expect(client('b').getMailboxes).toHaveBeenCalledTimes(2); }); + it('tries a login again when its first folder load failed', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + connected(['a', 'b']); + client('b').getMailboxes.mockRejectedValueOnce(new Error('offline')); + renderHook(() => useProMultiAccountMailboxes()); + await waitFor(() => expect(client('b').getMailboxes).toHaveBeenCalledTimes(1)); + await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); + act(() => connected(['a', 'b', 'c'])); + await waitFor(() => expect(useEmailStore.getState().accountMailboxes.b).toBeDefined()); + expect(client('b').getMailboxes).toHaveBeenCalledTimes(2); + }); + it('stays idle outside the Pro shell', () => { useSettingsStore.setState({ proInterface: false } as never); connected(['a', 'b']); diff --git a/hooks/use-pro-multi-account-mailboxes.ts b/hooks/use-pro-multi-account-mailboxes.ts index c3ffd734a..96f38c954 100644 --- a/hooks/use-pro-multi-account-mailboxes.ts +++ b/hooks/use-pro-multi-account-mailboxes.ts @@ -53,7 +53,10 @@ export function useProMultiAccountMailboxes(): void { const client = getClientForAccount(id); if (!client || loaded.current.get(id) === client) continue; loaded.current.set(id, client); - void loadAccountMailboxes(client, id); + void loadAccountMailboxes(client, id).then((ok) => { + // Try a failed login again on the next run, as the identities hook does. + if (!ok && loaded.current.get(id) === client) loaded.current.delete(id); + }); } }, [proInterface, isEmbedded, connectedSignature, activeAccountId]); } diff --git a/stores/__tests__/email-store-unified-mailbox-cache.test.ts b/stores/__tests__/email-store-unified-mailbox-cache.test.ts index 6735757ed..0b7959bff 100644 --- a/stores/__tests__/email-store-unified-mailbox-cache.test.ts +++ b/stores/__tests__/email-store-unified-mailbox-cache.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { buildUnifiedAccountClients, invalidateUnifiedMailboxes, useEmailStore } from '../email-store'; +import { buildUnifiedAccountClients, invalidateUnifiedMailboxes, loadAccountMailboxes, useEmailStore } from '../email-store'; import { useAccountStore } from '../account-store'; import { useAuthStore } from '../auth-store'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; @@ -109,6 +109,16 @@ describe('unified scope folder lists', () => { expect(useEmailStore.getState().accountMailboxes.a[0].unreadEmails).toBe(4); }); + it('publishes a list the Pro shell loaded under the JMAP id too', async () => { + const b = fakeClient('b'); + connect({ b }); + await loadAccountMailboxes(b as unknown as IJMAPClient, 'b'); + await buildUnifiedAccountClients(); + expect(b.getMailboxes).toHaveBeenCalledTimes(1); + // Actions on b's mail resolve its folders by JMAP id. + expect(useEmailStore.getState().accountMailboxes['jmap-b']?.[0].id).toBe('b-inbox'); + }); + it('retries a login whose fetch failed', async () => { const a = fakeClient('a'); a.getMailboxes.mockRejectedValueOnce(new Error('offline')); diff --git a/stores/email-store.ts b/stores/email-store.ts index c91b62655..2c89261ab 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -1065,15 +1065,17 @@ async function unifiedMailboxesFor( * the unified scope uses, so the two never fetch the same list twice and a * list already there is not fetched again until that login reports a change. */ -export async function loadAccountMailboxes(client: IJMAPClient, accountId: string): Promise { +export async function loadAccountMailboxes(client: IJMAPClient, accountId: string): Promise { try { const { mailboxes, fresh } = await unifiedMailboxesFor(client, false); - if (!fresh && useEmailStore.getState().accountMailboxes[accountId]) return; + if (!fresh && useEmailStore.getState().accountMailboxes[accountId]) return true; useEmailStore.setState((state) => ({ accountMailboxes: { ...state.accountMailboxes, [accountId]: mailboxes.slice() }, })); + return true; } catch (error) { console.error(`Failed to fetch mailboxes for account ${accountId}:`, error); + return false; } } @@ -1110,6 +1112,10 @@ export async function buildUnifiedAccountClients( // Skip the account on mailbox fetch failure. if (result.status !== 'fulfilled' || !result.value) continue; const { a, c, mailboxes, fresh } = result.value; + // A reused list never overwrites a live one, but fills a key nothing has published yet. + const publish = (key: string, list: Mailbox[]) => { + if (fresh || !useEmailStore.getState().accountMailboxes[key]) fetchedMailboxes[key] = list; + }; try { const ownMailboxes = includeGroup ? mailboxes.filter((m) => !m.isShared) @@ -1120,12 +1126,10 @@ export async function buildUnifiedAccountClients( // resolution branch-free against shared sources. const primaryJmapId = c.getAccountId(); built.push({ accountId: a.id, accountLabel: a.label || a.email, client: c, mailboxes: ownMailboxes, clientAccountId: a.id, jmapAccountId: primaryJmapId, isShared: false, crossIncludedMailboxIds: resolveCrossIncludedMailboxIds(a.id, ownMailboxes) }); - if (fresh) { - fetchedMailboxes[a.id] = ownMailboxes; - // Also cache under the JMAP id so `accountMailboxes[email.sourceAccountId]` - // resolves uniformly for personal and shared sources alike. - fetchedMailboxes[primaryJmapId] = ownMailboxes; - } + publish(a.id, ownMailboxes); + // Also cache under the JMAP id so `accountMailboxes[email.sourceAccountId]` + // resolves uniformly for personal and shared sources alike. + publish(primaryJmapId, ownMailboxes); if (includeGroup) { const sharedByOwner = new Map(); @@ -1149,7 +1153,7 @@ export async function buildUnifiedAccountClients( // Cache the owner's mailbox list keyed by its JMAP id so single-email // and batch actions can resolve role-based destinations (trash/archive) // in the owner account instead of falling back to the active account. - if (fresh) fetchedMailboxes[ownerId] = ownerMailboxes; + publish(ownerId, ownerMailboxes); } } } catch {