From c08375974ac28540227c481e4ed62dafb213c4a4 Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Fri, 25 Sep 2026 07:30:55 +1200 Subject: [PATCH] fix(email-store): stop late unified/cross-view fetches from stomping folder selection :bug: fetchUnifiedEmails and fetchCrossView set isUnifiedView:true synchronously then await a network fetch with no check on completion that the view is still current. If one resolved after the user had since selected a real folder, it re-set isUnifiedView back to true while selectedMailbox stayed on the folder - so a push-driven refreshCurrentMailbox (e.g. after deleting a message) took the unified branch and blended in Inbox-tagged messages instead of refreshing the folder. Add a viewToken bumped by selectMailbox/selectKeyword/exitUnifiedView; fetchUnifiedEmails/fetchCrossView capture it before fetching and discard their result if it no longer matches on return. Also make selectMailbox/ selectKeyword defensively reset isUnifiedView/unifiedRole/crossView. Upstream: bulwarkmail/webmail#1102 --- .../email-store-view-token-race.test.ts | 112 ++++++++++++++++++ stores/email-store.ts | 39 +++++- 2 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 stores/__tests__/email-store-view-token-race.test.ts diff --git a/stores/__tests__/email-store-view-token-race.test.ts b/stores/__tests__/email-store-view-token-race.test.ts new file mode 100644 index 000000000..472de927b --- /dev/null +++ b/stores/__tests__/email-store-view-token-race.test.ts @@ -0,0 +1,112 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +/** + * Deleting a message in a real folder (e.g. one built from a Sieve `fileinto` + * rule) could show "Inbox" badges and stale rows blended in, because a + * still-in-flight fetchUnifiedEmails/fetchCrossView call (kicked off before + * the user navigated to that folder) resolved AFTER selectMailbox and + * unconditionally stomped isUnifiedView back to true. The next + * refreshCurrentMailbox (triggered by the delete's push echo) then took the + * unified branch instead of refreshing the real folder. + * + * viewToken is bumped by every view-changing action; fetchUnifiedEmails and + * fetchCrossView capture it before their async fetch and bail if it no + * longer matches on return, so a late response can't clobber a newer view. + */ + +const { fetchCrossViewMock, fetchUnifiedMock } = vi.hoisted(() => ({ + fetchCrossViewMock: vi.fn(), + fetchUnifiedMock: vi.fn(), +})); + +vi.mock('@/lib/unified-mailbox', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + fetchCrossViewEmails: fetchCrossViewMock, + fetchUnifiedEmails: fetchUnifiedMock, + }; +}); + +import { useEmailStore } from '../email-store'; +import { useSettingsStore } from '../settings-store'; +import type { Email } from '@/lib/jmap/types'; +import type { UnifiedAccountClient } from '@/lib/unified-mailbox'; + +const makeEmail = (id: string): 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; + +describe('viewToken guards against late-resolving unified fetches (#delete-folder-blend)', () => { + let resolveUnified!: (value: unknown) => void; + + beforeEach(() => { + vi.clearAllMocks(); + useSettingsStore.setState({ emailsPerPage: 25 }); + useEmailStore.setState({ + emails: [], + totalEmails: 0, + isUnifiedView: false, + unifiedRole: null, + crossView: null, + selectedMailbox: '', + selectedKeyword: null, + }); + fetchUnifiedMock.mockImplementation( + () => new Promise((resolve) => { resolveUnified = resolve; }), + ); + }); + + it('a late-resolving fetchUnifiedEmails does not stomp a folder selected afterward', async () => { + const accounts: UnifiedAccountClient[] = []; + + // Kick off a unified fetch and don't await it yet - simulates it being + // in flight when the user clicks away. + const pending = useEmailStore.getState().fetchUnifiedEmails(accounts, 'inbox'); + expect(useEmailStore.getState().isUnifiedView).toBe(true); + + // User navigates to a real folder (e.g. "Later") before the fetch settles. + useEmailStore.getState().selectMailbox('later-folder-id'); + expect(useEmailStore.getState().isUnifiedView).toBe(false); + expect(useEmailStore.getState().selectedMailbox).toBe('later-folder-id'); + + // The stale unified fetch now resolves. + resolveUnified({ + emails: [makeEmail('unified-a'), makeEmail('unified-b')], + hasMore: false, + total: 2, + errors: new Map(), + }); + await pending; + + // It must not have re-entered unified view or overwritten the folder's state. + expect(useEmailStore.getState().isUnifiedView).toBe(false); + expect(useEmailStore.getState().selectedMailbox).toBe('later-folder-id'); + expect(useEmailStore.getState().emails.map((e) => e.id)).not.toContain('unified-a'); + }); + + it('a fresh fetchUnifiedEmails call still applies its own results normally', async () => { + fetchUnifiedMock.mockResolvedValueOnce({ + emails: [makeEmail('a')], + hasMore: false, + total: 1, + errors: new Map(), + }); + + await useEmailStore.getState().fetchUnifiedEmails([], 'inbox'); + + expect(useEmailStore.getState().isUnifiedView).toBe(true); + expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['a']); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index 88365d9a0..401bac775 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -121,6 +121,11 @@ interface EmailStore { externalSearchResults: ExternalSearchResult[]; // Unified mailbox state + // Bumped by any action that changes which mailbox/keyword/unified view is + // active. fetchUnifiedEmails/fetchCrossView capture it before their async + // fetch and check it on return, so a late-resolving fetch from a view the + // user already navigated away from can't stomp the newer view's state (#delete-folder-blend). + viewToken: number; isUnifiedView: boolean; unifiedRole: UnifiedMailboxRole | null; // Cross-account view ('unread' | 'starred' | 'all') when active; null for the @@ -1432,6 +1437,7 @@ export const useEmailStore = create((set, get) => ({ externalSearchResults: [], // Unified mailbox state + viewToken: 0, isUnifiedView: false, unifiedRole: null, crossView: null, @@ -1516,7 +1522,7 @@ export const useEmailStore = create((set, get) => ({ emailHooks.onEmailOpen.emitSync(email); } }, - selectKeyword: (keyword) => set({ + selectKeyword: (keyword) => set(state => ({ selectedKeyword: keyword, isLoadingMore: false, selectedEmail: null, @@ -1524,7 +1530,11 @@ export const useEmailStore = create((set, get) => ({ expandedThreadIds: new Set(), threadEmailsCache: new Map(), threadEmailCounts: new Map(), - }), + viewToken: state.viewToken + 1, + isUnifiedView: false, + unifiedRole: null, + crossView: null, + })), fetchTagCounts: (client) => coalesceRefresh(client, 'tagCounts', async () => { try { const keywords = useSettingsStore.getState().emailKeywords; @@ -1553,7 +1563,7 @@ export const useEmailStore = create((set, get) => ({ console.error('Failed to fetch tag counts:', error); } }), - selectMailbox: (mailboxId) => set({ + selectMailbox: (mailboxId) => set(state => ({ selectedMailbox: mailboxId, isLoadingMore: false, selectedEmail: null, @@ -1563,7 +1573,11 @@ export const useEmailStore = create((set, get) => ({ threadEmailsCache: new Map(), threadEmailCounts: new Map(), isLoadingThread: null, - }), + viewToken: state.viewToken + 1, + isUnifiedView: false, + unifiedRole: null, + crossView: null, + })), setLoading: (loading) => set({ isLoading: loading }), setLoadingEmail: (loading) => set({ isLoadingEmail: loading }), setError: (error) => set({ error }), @@ -4777,6 +4791,7 @@ export const useEmailStore = create((set, get) => ({ // Unified mailbox operations fetchUnifiedEmails: async (accounts, role) => { + const token = get().viewToken + 1; set({ isLoading: true, error: null, @@ -4785,11 +4800,15 @@ export const useEmailStore = create((set, get) => ({ crossView: null, selectedKeyword: null, retainedInViewIds: new Set(), + viewToken: token, }); try { const emailsPerPage = useSettingsStore.getState().emailsPerPage; const order = getMessageListOrderFor(role); const result = await fetchUnifiedEmails(accounts, role, emailsPerPage, 0, order); + // The user may have navigated to a different mailbox/keyword/unified view + // while this was in flight — don't let a stale response overwrite it. + if (get().viewToken !== token) return; set({ emails: result.emails, hasMoreEmails: result.hasMore, @@ -4799,6 +4818,7 @@ export const useEmailStore = create((set, get) => ({ unifiedErrors: result.errors, }); } catch (error) { + if (get().viewToken !== token) return; console.error('Failed to fetch unified emails:', error); set({ error: error instanceof Error ? error.message : "Failed to fetch unified emails", @@ -4855,6 +4875,7 @@ export const useEmailStore = create((set, get) => ({ }, fetchCrossView: async (accounts, view) => { + const token = get().viewToken + 1; set({ isLoading: true, error: null, @@ -4863,10 +4884,14 @@ export const useEmailStore = create((set, get) => ({ crossView: view, selectedKeyword: null, retainedInViewIds: new Set(), + viewToken: token, }); try { const emailsPerPage = useSettingsStore.getState().emailsPerPage; const result = await fetchCrossViewEmails(accounts, view, emailsPerPage, 0); + // The user may have navigated to a different mailbox/keyword/unified view + // while this was in flight — don't let a stale response overwrite it. + if (get().viewToken !== token) return; set({ emails: result.emails, hasMoreEmails: result.hasMore, @@ -4875,6 +4900,7 @@ export const useEmailStore = create((set, get) => ({ unifiedErrors: result.errors, }); } catch (error) { + if (get().viewToken !== token) return; console.error('Failed to fetch cross-account view:', error); set({ error: error instanceof Error ? error.message : "Failed to fetch cross-account view", @@ -4898,13 +4924,14 @@ export const useEmailStore = create((set, get) => ({ }, exitUnifiedView: () => { - set({ + set(state => ({ isUnifiedView: false, unifiedRole: null, crossView: null, unifiedErrors: new Map(), retainedInViewIds: new Set(), - }); + viewToken: state.viewToken + 1, + })); }, setScheduledView: (isScheduledView, accountScope = null) => set(state => {