From e98a0e6351ea5262bae5d048a3c75c04a3e3604d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 15:31:05 +0200 Subject: [PATCH] fix: drop folder and filter fetches that finish after an account switch A folder or filter fetch started for one account and answered after a switch wrote its result into the stores the next account had just filled. The sidebar then showed the old account's folders, and the filter store kept the old account's rules and script id, so the next save uploaded them - a redirect to someone else's address included - to the new account's server. Clearing the stores for another account now bumps a store epoch. `fetchMailboxes` and the email list's view check drop results from an older epoch, and the filter store drops any fetch that is no longer the latest one. --- lib/account-state-manager.ts | 4 + lib/store-epoch.ts | 16 ++++ .../store-fetch-after-switch.test.ts | 80 +++++++++++++++++++ stores/email-store.ts | 12 ++- stores/filter-store.ts | 48 +++++++---- 5 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 lib/store-epoch.ts create mode 100644 stores/__tests__/store-fetch-after-switch.test.ts diff --git a/lib/account-state-manager.ts b/lib/account-state-manager.ts index 92fdb092d..33840c9fd 100644 --- a/lib/account-state-manager.ts +++ b/lib/account-state-manager.ts @@ -12,6 +12,7 @@ import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils'; import { useIdentityStore } from '@/stores/identity-store'; import { useVacationStore } from '@/stores/vacation-store'; import { useFileStore } from '@/stores/file-store'; +import { bumpStoreEpoch } from '@/lib/store-epoch'; // Minimal snapshot shapes - we only capture what we need // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -105,6 +106,9 @@ export function restoreAccount(accountId: string): boolean { /** Clear all stores (used before restoring a different account) */ export function clearAllStores(): void { + // In-flight fetches of the outgoing account must not land in the stores + // the next account is about to fill. + bumpStoreEpoch(); useEmailStore.setState({ emails: [], mailboxes: [], diff --git a/lib/store-epoch.ts b/lib/store-epoch.ts new file mode 100644 index 000000000..fb3c63b8b --- /dev/null +++ b/lib/store-epoch.ts @@ -0,0 +1,16 @@ +/** + * Bumped whenever the account-scoped stores are emptied for another account + * (an account switch, a sign-out). A fetch captures it before awaiting the + * server and drops its result if it has changed since: the data belongs to + * an account that is no longer on screen, and writing it would show - and + * let the user act on - one account's folders or filters in another. + */ +let epoch = 0; + +export function currentStoreEpoch(): number { + return epoch; +} + +export function bumpStoreEpoch(): void { + epoch++; +} diff --git a/stores/__tests__/store-fetch-after-switch.test.ts b/stores/__tests__/store-fetch-after-switch.test.ts new file mode 100644 index 000000000..f8a4a8e0d --- /dev/null +++ b/stores/__tests__/store-fetch-after-switch.test.ts @@ -0,0 +1,80 @@ +import { expect, it } from 'vitest'; + +// Store fetches had no account guard: a request started for account A that +// resolved after the switch to B wrote A's folders into B's sidebar, and A's +// filter rules and script id into B's filter store - so B's next save +// uploaded A's rules (a redirect to A's bookkeeper included) to B's server. +import { useEmailStore } from '@/stores/email-store'; +import { useFilterStore } from '@/stores/filter-store'; +import { useAuthStore } from '@/stores/auth-store'; +import { clearAllStores } from '@/lib/account-state-manager'; + +function deferred() { + let resolve!: (v: T) => void; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +it('fetchMailboxes for A landing after the switch leaves B folder tree alone', async () => { + const gate = deferred(); + const aMailboxes = [ + { id: 'a', name: 'Inbox', role: 'inbox', accountId: 'A', totalEmails: 1, unreadEmails: 0, sortOrder: 0 }, + { id: 'g', name: 'A-Invoices', role: null, accountId: 'A', totalEmails: 1, unreadEmails: 0, sortOrder: 1 }, + ]; + const clientA = { + getAllMailboxesWithState: async () => { await gate.promise; return { mailboxes: aMailboxes, states: { A: 's1' } }; }, + }; + useAuthStore.setState({ activeAccountId: 'acct-A' }); + useEmailStore.setState({ mailboxes: [{ id: 'a', name: 'Inbox', role: 'inbox', accountId: 'A' }] as never, selectedMailbox: 'a' }); + const p = useEmailStore.getState().fetchMailboxes(clientA as never); // e.g. push-driven refresh for A + + // User switches to B: stores reset and B's folders restored. + clearAllStores(); + useAuthStore.setState({ activeAccountId: 'acct-B' }); + const bMailboxes = [ + { id: 'a', name: 'Inbox', role: 'inbox', accountId: 'B' }, + { id: 'g', name: 'B-Family', role: null, accountId: 'B' }, + ]; + useEmailStore.setState({ mailboxes: bMailboxes as never, selectedMailbox: 'a' }); + + gate.resolve(); + await p; + const names = useEmailStore.getState().mailboxes.map((m) => `${m.id}:${m.name}`); + expect(names).toEqual(['a:Inbox', 'g:B-Family']); +}); + +it('filter fetch for A landing after B loaded is dropped', async () => { + const gateA = deferred(); + const scriptA = 'require ["fileinto"];\r\nif header :contains "subject" "invoice" { redirect "bookkeeper@a.example"; }\r\n'; + const scriptB = 'require ["fileinto"];\r\nif header :contains "from" "mom@b.example" { fileinto "Family"; }\r\n'; + const mk = (sieveAcct: string, script: string, gate?: Promise) => { + const uploads: Array<{ id: string; content: string; account: string | undefined }> = []; + return { + uploads, + client: { + getSieveAccounts: () => [{ id: sieveAcct, name: sieveAcct, isPrimary: true }], + getSieveAccountId: () => sieveAcct, + getSieveCapabilities: () => ({ sieveExtensions: ['fileinto', 'include'] }), + getSieveScripts: async () => { if (gate) await gate; return [{ id: sieveAcct === 'sA' ? 'a' : 'c', name: 'filters', blobId: 'blob-' + sieveAcct, isActive: true }]; }, + getSieveScriptContent: async () => script, + updateSieveScript: async (id: string, content: string, _act: boolean, account?: string) => { uploads.push({ id, content, account }); }, + createSieveScript: async () => ({ id: 'new' }), + }, + }; + }; + const A = mk('sA', scriptA, gateA.promise); + const B = mk('sB', scriptB); + + const fA = useFilterStore.getState().fetchFilters(A.client as never); // in flight for A + useFilterStore.getState().clearState(); // switch → clearAllStores + await useFilterStore.getState().selectAccount(B.client as never, 'sB'); // B's filters page loads + gateA.resolve(); + await fA; + const s = useFilterStore.getState(); + expect(s.selectedAccountId).toBe('sB'); + expect(s.activeScriptId).toBe('c'); + expect(s.rawScript).not.toContain('bookkeeper@a.example'); + await useFilterStore.getState().saveFilters(B.client as never); // user toggles a rule in B + expect(B.uploads[0].account).toBe('sB'); + expect(B.uploads[0].content).not.toContain('bookkeeper@a.example'); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index c17b480e9..d6647a5b2 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -12,6 +12,7 @@ import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils"; import type { ExternalSearchResult } from "@/lib/plugin-types"; import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; import { useAuthStore } from "@/stores/auth-store"; +import { currentStoreEpoch } from "@/lib/store-epoch"; import { useAccountStore } from "@/stores/account-store"; import { useMessageListTabsStore } from "@/stores/message-list-tabs-store"; @@ -699,9 +700,11 @@ function resolveActionMailboxes(): Mailbox[] { function captureEmailListView(): () => boolean { const view = useEmailStore.getState(); const activeAccountId = useAuthStore.getState().activeAccountId; + const epoch = currentStoreEpoch(); return () => { const current = useEmailStore.getState(); - return current.selectedMailbox === view.selectedMailbox + return currentStoreEpoch() === epoch + && current.selectedMailbox === view.selectedMailbox && current.selectedKeyword === view.selectedKeyword && current.viewingAccountId === view.viewingAccountId && current.isUnifiedView === view.isUnifiedView @@ -1662,6 +1665,10 @@ export const useEmailStore = create((set, get) => ({ // JMAP operations fetchMailboxes: (client) => coalesceRefresh(client, 'mailboxes', async () => { + // An account switch while this is in flight empties the stores for the + // next account; this account's folders must not land in them. + const epoch = currentStoreEpoch(); + const stale = () => currentStoreEpoch() !== epoch; // Only toggle the email list's isLoading on the initial load. Background // refreshes (after a move/archive that may have created new folders) must // not flash the list's loading state, which hides the results-count bar @@ -1674,6 +1681,7 @@ export const useEmailStore = create((set, get) => ({ const fetched = client.getAllMailboxesWithState ? await client.getAllMailboxesWithState() : { mailboxes: await client.getAllMailboxes(), states: {} }; + if (stale()) return; const mailboxes = fetched.mailboxes; // Guard against a transient fetch returning an empty list (e.g. a server @@ -1688,6 +1696,7 @@ export const useEmailStore = create((set, get) => ({ // fetch. This is also emitted for Mailbox-only state changes (for example // an empty provider label), where no Email fetch follows. await emailHooks.onMailboxesRefresh.emit(mailboxes); + if (stale()) return; // Auto-select inbox if no mailbox is selected or the current selection // doesn't exist in the fetched list (e.g. after an account switch) @@ -1715,6 +1724,7 @@ export const useEmailStore = create((set, get) => ({ set({ mailboxes, ...loadingPatch }); } } catch (error) { + if (stale()) return; // The existing folder list is deliberately left untouched: a failed // background refresh must never blank the sidebar (#780). set({ diff --git a/stores/filter-store.ts b/stores/filter-store.ts index 3de5c50ec..e4afa0617 100644 --- a/stores/filter-store.ts +++ b/stores/filter-store.ts @@ -43,6 +43,14 @@ interface FilterStore { clearState: () => void; } +/** + * Bumped by every fetch, account selection and reset. A fetch that is no + * longer the latest when its answer arrives drops it: an account switch + * during the fetch would otherwise leave one account's rules and script id + * in place for the next account, and the next save would upload them there. + */ +let fetchGeneration = 0; + export const useFilterStore = create()((set, get) => ({ rules: [], isLoading: false, @@ -62,6 +70,8 @@ export const useFilterStore = create()((set, get) => ({ setSupported: (supported) => set({ isSupported: supported }), fetchFilters: async (client, accountId) => { + const generation = ++fetchGeneration; + const stale = () => generation !== fetchGeneration; set({ isLoading: true, error: null }); try { const accounts = client.getSieveAccounts(); @@ -73,6 +83,7 @@ export const useFilterStore = create()((set, get) => ({ set({ sieveCapabilities: capabilities }); const allScripts = await client.getSieveScripts(resolvedId); + if (stale()) return; debug.log('filters', 'Sieve scripts fetched:', allScripts.length); // Skip the server-managed 'vacation' script (RFC 9661 §4) - it can only @@ -101,6 +112,7 @@ export const useFilterStore = create()((set, get) => ({ set({ activeScriptId: activeScript.id }); const content = await client.getSieveScriptContent(activeScript.blobId, resolvedId); + if (stale()) return; set({ rawScript: content }); const result = parseScript(content); @@ -127,6 +139,7 @@ export const useFilterStore = create()((set, get) => ({ }); } } catch (error) { + if (stale()) return; debug.error('Failed to fetch filters:', error); set({ isLoading: false, @@ -252,22 +265,25 @@ export const useFilterStore = create()((set, get) => ({ resetToVisualBuilder: () => set({ isOpaque: false, rawScript: '', rules: [], externalRequires: [] }), - clearState: () => set({ - rules: [], - isLoading: false, - isSaving: false, - error: null, - isSupported: false, - sieveCapabilities: null, - activeScriptId: null, - isOpaque: false, - rawScript: '', - vacationSettings: null, - externalRequires: [], - includeVacation: false, - availableAccounts: [], - selectedAccountId: null, - }), + clearState: () => { + fetchGeneration++; + set({ + rules: [], + isLoading: false, + isSaving: false, + error: null, + isSupported: false, + sieveCapabilities: null, + activeScriptId: null, + isOpaque: false, + rawScript: '', + vacationSettings: null, + externalRequires: [], + includeVacation: false, + availableAccounts: [], + selectedAccountId: null, + }); + }, })); function supportsInclude(capabilities: SieveCapabilities | null): boolean {