diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index 26352683..94d884e6 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -1001,29 +1001,52 @@ export default function Home() { }; }, [isAuthenticated, client, fetchMailboxes, fetchEmails, fetchQuota, fetchTagCounts, refreshScheduledMetadata]); - // Push notifications: set up once per client and tear down when the client - // goes away (logout or account switch). Kept separate from the fetch effect - // above so it still runs when data was prefetched at login time. + // Push notifications: set up once per CONNECTED client and tear down when the + // clients go away (logout or account switch). Kept separate from the fetch + // effect above so it still runs when data was prefetched at login time. + // + // We bind every connected login, not just the active one: background accounts + // must drive the unified-section counters too. The active client keeps the + // full handler (current list / scheduled / calendar / filters); background + // logins only re-project the unified counts by rebuilding the unified scope + // (which refreshes every account's cached mailbox list), since their changes + // never touch the active `mailboxes`. (#281 background push) useEffect(() => { if (!isAuthenticated || !client) return; - try { - client.onStateChange((change) => handleStateChange(change, client)); - const pushEnabled = client.setupPushNotifications(); - if (pushEnabled) { - setPushConnected(true); - debug.log('push', '[Push] Push notifications successfully enabled'); - } else { - debug.log('push', '[Push] Push notifications not available on this server'); + const clients = useAuthStore.getState().getAllConnectedClients(); + const cleanups: Array<() => void> = []; + + for (const [accId, c] of clients) { + try { + if (accId === activeAccountId) { + c.onStateChange((change) => handleStateChange(change, c)); + } else { + c.onStateChange(() => { + buildPopulatedUnifiedAccounts() + .then((built) => { + refreshCrossCounts(built); + refreshUnifiedCounts(built); + }) + .catch(() => { /* per-account fetch failures surface elsewhere */ }); + }); + } + c.setupPushNotifications(); + cleanups.push(() => c.closePushNotifications()); + } catch (error) { + debug.log('push', '[Push] Failed to setup push notifications for account:', accId, error); } - } catch (error) { - debug.log('push', '[Push] Failed to setup push notifications:', error); + } + + if (cleanups.length > 0) { + setPushConnected(true); + debug.log('push', `[Push] Push notifications enabled for ${cleanups.length} account(s)`); } return () => { - client.closePushNotifications(); + cleanups.forEach((fn) => fn()); }; - }, [isAuthenticated, client, handleStateChange, setPushConnected]); + }, [isAuthenticated, client, activeAccountId, connectedAccountsSignature, handleStateChange, setPushConnected, buildPopulatedUnifiedAccounts, refreshCrossCounts, refreshUnifiedCounts]); // Keep unified mailbox counts in sync when the feature is enabled and more // than one account is connected. Runs whenever the set of connected accounts diff --git a/lib/__tests__/jmap-client-resilience.test.ts b/lib/__tests__/jmap-client-resilience.test.ts index 9d8c9388..cd13d5cb 100644 --- a/lib/__tests__/jmap-client-resilience.test.ts +++ b/lib/__tests__/jmap-client-resilience.test.ts @@ -397,4 +397,53 @@ describe('JMAPClient resilience', () => { ).rejects.toThrow('Failed to fetch blob: 404'); }); }); + + // #281 V3: every email fetch path must namespace mailboxIds for shared/ + // delegated accounts (`${ownerId}:${id}`) so they line up with the store's + // namespaced shared-mailbox ids. searchEmails/advancedSearchEmails are the + // cross-view (All mail / Unread / Starred) browse paths and previously did not. + describe('shared-account mailboxId namespacing', () => { + function queryAndGet(email: Record) { + return { + methodResponses: [ + ['Email/query', { total: 1, ids: ['e1'] }, '0'], + ['Email/get', { list: [email] }, '1'], + ], + }; + } + + it('advancedSearchEmails namespaces bare owner mailboxIds for a foreign account', async () => { + const client = await createConnectedClient(); // primary acct-1 + fetchSpy.mockResolvedValueOnce( + mockFetchResponse(200, queryAndGet({ id: 'e1', receivedAt: '2026-01-01T00:00:00Z', mailboxIds: { 'x-inbox': true } })), + ); + + const { emails } = await client.advancedSearchEmails({ inMailbox: 'owner-x:x-inbox' }, 'owner-x'); + + expect(emails[0].mailboxIds).toEqual({ 'owner-x:x-inbox': true }); + expect(emails[0].mailboxIds['x-inbox']).toBeUndefined(); + }); + + it('searchEmails namespaces bare owner mailboxIds for a foreign account', async () => { + const client = await createConnectedClient(); + fetchSpy.mockResolvedValueOnce( + mockFetchResponse(200, queryAndGet({ id: 'e1', receivedAt: '2026-01-01T00:00:00Z', mailboxIds: { 'x-inbox': true } })), + ); + + const { emails } = await client.searchEmails('hello', undefined, 'owner-x'); + + expect(emails[0].mailboxIds).toEqual({ 'owner-x:x-inbox': true }); + }); + + it('leaves own-account mailboxIds untouched (no foreign accountId)', async () => { + const client = await createConnectedClient(); // primary acct-1 + fetchSpy.mockResolvedValueOnce( + mockFetchResponse(200, queryAndGet({ id: 'e1', receivedAt: '2026-01-01T00:00:00Z', mailboxIds: { inbox: true } })), + ); + + const { emails } = await client.advancedSearchEmails({ inMailbox: 'inbox' }); + + expect(emails[0].mailboxIds).toEqual({ inbox: true }); + }); + }); }); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 387f2393..04aabcdc 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1885,6 +1885,13 @@ export class JMAPClient implements IJMAPClient { const total = queryResponse?.total || 0; const hasMore = computeHasMore(position, emails.length, total, limit); + // Mirror getEmails: emails fetched from a delegated/shared account carry + // bare owner mailbox ids; namespace them to `${ownerId}:${id}` so they line + // up with the namespaced ids the store holds for shared mailboxes. (#281 V3) + if (accountId && accountId !== this.accountId) { + namespaceMailboxIds(emails, accountId); + } + return { emails, hasMore, total }; } catch (error) { console.error('Search failed:', error); @@ -1925,6 +1932,13 @@ export class JMAPClient implements IJMAPClient { const total = queryResponse?.total || 0; const hasMore = computeHasMore(position, emails.length, total, limit); + // Namespace shared/delegated-account mailbox ids (see searchEmails). The + // cross-account views (All mail / Unread / Starred) browse via this method, + // so without it shared emails would carry bare owner ids there. (#281 V3) + if (accountId && accountId !== this.accountId) { + namespaceMailboxIds(emails, accountId); + } + return { emails, hasMore, total }; } catch (error) { console.error('Advanced search failed:', error); diff --git a/lib/unified-mailbox.ts b/lib/unified-mailbox.ts index 81804dc7..e5e91e23 100644 --- a/lib/unified-mailbox.ts +++ b/lib/unified-mailbox.ts @@ -61,7 +61,12 @@ const ALL_UNIFIED_ROLES: UnifiedMailboxRole[] = [ */ export function resolveSourceFolderName(email: Email, mailboxes: Mailbox[]): string | undefined { for (const m of mailboxes) { - if (email.mailboxIds?.[m.originalId ?? m.id]) return m.name; + // All fetch paths now namespace shared emails' mailboxIds to the store id + // (`${ownerId}:${origId}`), so matching `m.id` works for own and shared + // alike. The `originalId` check stays as a defensive fallback for any email + // that still carries a bare owner id. (#281 V3) + if (email.mailboxIds?.[m.id]) return m.name; + if (m.originalId && email.mailboxIds?.[m.originalId]) return m.name; } return undefined; } diff --git a/stores/__tests__/email-store-unified-actions.test.ts b/stores/__tests__/email-store-unified-actions.test.ts index 38309b59..68a782b8 100644 --- a/stores/__tests__/email-store-unified-actions.test.ts +++ b/stores/__tests__/email-store-unified-actions.test.ts @@ -3,6 +3,7 @@ import { useEmailStore } from '../email-store'; import { useAuthStore } from '../auth-store'; import type { Email, Mailbox } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import type { UnifiedAccountClient } from '@/lib/unified-mailbox'; // Regression coverage for issue #281: single-email actions performed in the // unified inbox must be routed to the *email's own account* client, not the @@ -98,6 +99,9 @@ describe('unified-view single-email action routing (#281)', () => { processingReadStatus: new Set(), selectedEmail: null, selectedEmailIds: new Set(), + unifiedScope: [], + unifiedCounts: [], + crossUnreadCount: 0, emails: [ // Second direct login: sourceClientAccountId === sourceAccountId === 'account-b'. makeEmail({ id: 'email-b', accountId: 'account-b', sourceClientAccountId: 'account-b', sourceAccountId: 'account-b', keywords: {}, mailboxIds: { 'b-inbox': true } }), @@ -216,6 +220,62 @@ describe('unified-view single-email action routing (#281)', () => { expect(shared.totalEmails).toBe(9); // was 10 }); + it('decrements the unified-section badges when deleting from the unified view (live projection)', async () => { + // The unified-section badges (unifiedCounts / crossUnreadCount) must be a + // live projection of the per-account mailbox lists, NOT a stale server + // snapshot. Deleting a message in the unified view patches the folder's + // counter; the badge must follow in lockstep without a re-fetch. + const scope: UnifiedAccountClient[] = [ + { + accountId: 'account-a', accountLabel: 'A', client: activeClient, + clientAccountId: 'account-a', jmapAccountId: 'account-a', isShared: false, + mailboxes: [makeMailbox({ id: 'a-inbox', role: 'inbox' })], + }, + { + accountId: 'owner-x', accountLabel: 'Shared', client: activeClient, + clientAccountId: 'account-a', jmapAccountId: 'owner-x', isShared: true, + mailboxes: [makeMailbox({ id: 'owner-x:x-inbox', originalId: 'x-inbox', role: 'inbox', isShared: true, accountId: 'owner-x' })], + }, + ]; + + useEmailStore.setState({ + mailboxes: [ + makeMailbox({ id: 'a-inbox', role: 'inbox', unreadEmails: 2, totalEmails: 5 }), + makeMailbox({ + id: 'owner-x:x-inbox', originalId: 'x-inbox', name: 'Shared Inbox', + role: 'inbox', isShared: true, accountId: 'owner-x', + unreadEmails: 4, totalEmails: 10, + }), + ], + emails: [ + makeEmail({ + id: 'email-shared', accountId: 'owner-x', + sourceClientAccountId: 'account-a', sourceAccountId: 'owner-x', + keywords: {}, // unread + mailboxIds: { 'x-inbox': true }, // BARE owner id (not namespaced) + }), + ], + selectedEmailIds: new Set(['email-shared']), + }); + + // Seed the badges from the scope (also stores unifiedScope). + useEmailStore.getState().refreshUnifiedCounts(scope); + useEmailStore.getState().refreshCrossCounts(scope); + + const before = useEmailStore.getState(); + expect(before.unifiedCounts.find(c => c.role === 'inbox')).toMatchObject({ unreadEmails: 6, totalEmails: 15 }); + expect(before.crossUnreadCount).toBe(6); + + await useEmailStore.getState().batchDelete(activeClient, true); + + const after = useEmailStore.getState(); + // Underlying folder counter dropped... + expect(after.mailboxes.find(m => m.id === 'owner-x:x-inbox')!.unreadEmails).toBe(3); + // ...and the unified-section badges followed via the live projection. + expect(after.unifiedCounts.find(c => c.role === 'inbox')).toMatchObject({ unreadEmails: 5, totalEmails: 14 }); + expect(after.crossUnreadCount).toBe(5); + }); + it('still uses the active/passed client outside unified view', async () => { useEmailStore.setState({ isUnifiedView: false, diff --git a/stores/email-store.ts b/stores/email-store.ts index 8f3c9f7d..809e6f11 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -82,9 +82,23 @@ interface EmailStore { // isUnifiedView. crossView: CrossView | null; unifiedErrors: Map; // accountId -> error message + // Unified-section sidebar badges. These are NOT an independent source of + // truth: they are a pure projection of the live per-account mailbox lists + // (`mailboxes` + `accountMailboxes`) over the last-known unified scope + // (`unifiedScope`). Recomputed automatically whenever those lists change (see + // the store subscription below), so optimistic delete/move/markRead patches and + // push-driven mailbox refreshes flow into the badges without a server round + // trip. (#281 follow-up: single source of truth for unified counters.) unifiedCounts: UnifiedMailboxCounts[]; // Unread total across the cross-view included folders (badge for unread/all). crossUnreadCount: number; + // The account/folder structure (which accounts, role mailboxes, cross-include + // selection) that the unified badges are projected over. Set by + // refreshUnifiedCounts/refreshCrossCounts from the freshly-built + // UnifiedAccountClient[]. The COUNTER values it carries are ignored at + // projection time - live counters are read from `mailboxes`/`accountMailboxes` + // instead - so a stale snapshot here only affects structure, never numbers. + unifiedScope: UnifiedAccountClient[]; // Scheduled send state scheduledEmails: ScheduledEmail[]; @@ -508,15 +522,14 @@ async function refreshMailboxesForViewingAccount(fallbackClient: IJMAPClient): P // Whether an email belongs to a given mailbox, for local counter math. // Own-account emails carry bare ids (equal to both `id` and `originalId`). -// Shared/group emails fetched for the unified/cross views are decorated by -// lib/unified-mailbox.ts WITHOUT namespacing their `mailboxIds`, so they carry -// the owner's BARE JMAP ids while the shared mailbox is stored with a namespaced -// `id` (`${ownerId}:${origId}`), `isShared: true` and `originalId`/`accountId` -// (the owner). Matching only `mailbox.id` therefore missed every shared email, -// so a shared folder's counter never moved when mail was deleted/moved/read from -// the unified views. We match shared mailboxes via `originalId` too, but scope it -// to the owning account (`sourceAccountId === mailbox.accountId`) so a bare owner -// id can't collide with another account's folder. (#281, shared-counter fix) +// As of #281 V3, EVERY client fetch path (getEmails/getEmail/getThreadEmails/ +// searchEmails/advancedSearchEmails) namespaces shared/delegated emails' +// `mailboxIds` to the store id (`${ownerId}:${origId}`), so the `ids[mailbox.id]` +// fast path below matches own and shared mailboxes alike - one id space. +// The `originalId` branches remain as a defensive fallback for any email that +// still carries a bare owner id (scoped to the owning account via +// `sourceAccountId === mailbox.accountId` so a bare owner id can't collide with +// another account's folder). (#281) function emailInMailbox( email: { mailboxIds?: Record; sourceAccountId?: string }, mailbox: Mailbox, @@ -618,6 +631,62 @@ function applyDeleteCounters( }; } +// ─── Unified-badge live projection ──────────────────────────────────────────── +// +// The unified-section badges (unifiedCounts / crossUnreadCount) are a pure +// projection of the live per-account mailbox lists - the SAME lists the +// optimistic delete/move/markRead paths patch and that push refreshes. Rather +// than trusting the counter snapshot baked into the UnifiedAccountClient[] (which +// came from a server fetch and goes stale the moment a local mutation runs), we +// look each scope mailbox up by id in the live store list and use its current +// counters. This keeps the badges in lockstep with the per-folder counters - one +// source of truth, no server round trip, no eventual-consistency snap-back. + +// The live store list that holds a unified-scope account's folders. Mirrors +// applyMailboxCounterUpdate's routing exactly: the active client's folders (incl. +// its delegated shared folders) live in `mailboxes`; every other logged-in +// account's folders live in `accountMailboxes[clientAccountId]`. Falls back to +// the account's own (snapshot) list so an unknown account still contributes its +// last-known counters instead of vanishing. +function liveListForAccount( + account: UnifiedAccountClient, + state: { mailboxes: Mailbox[]; accountMailboxes: Record }, +): Mailbox[] { + const activeId = useAuthStore.getState().activeAccountId; + if (account.clientAccountId === activeId) return state.mailboxes; + return state.accountMailboxes[account.clientAccountId] ?? account.mailboxes; +} + +// Returns a shallow copy of the scope account whose mailboxes carry LIVE counter +// values (matched by id against the live store list). Structure - which +// mailboxes, their roles, originalId, crossIncludedMailboxIds - is preserved from +// the scope snapshot; only the counter numbers are refreshed. This lets the +// existing lib aggregators (fetchUnifiedMailboxCounts / getCrossUnreadTotal) run +// unchanged over live data. +function accountWithLiveCounters( + account: UnifiedAccountClient, + state: { mailboxes: Mailbox[]; accountMailboxes: Record }, +): UnifiedAccountClient { + const live = liveListForAccount(account, state); + const byId = new Map(live.map((m) => [m.id, m])); + return { ...account, mailboxes: account.mailboxes.map((m) => byId.get(m.id) ?? m) }; +} + +// Project the live mailbox state over the current unified scope into the two +// badge values. Pure function of (unifiedScope, mailboxes, accountMailboxes). +function projectUnifiedCounts( + state: { unifiedScope: UnifiedAccountClient[]; mailboxes: Mailbox[]; accountMailboxes: Record }, +): { unifiedCounts: UnifiedMailboxCounts[]; crossUnreadCount: number } { + if (state.unifiedScope.length === 0) { + return { unifiedCounts: [], crossUnreadCount: 0 }; + } + const live = state.unifiedScope.map((a) => accountWithLiveCounters(a, state)); + return { + unifiedCounts: fetchUnifiedMailboxCounts(live), + crossUnreadCount: getCrossUnreadTotal(live), + }; +} + // Find the trash mailbox for a given account scope. Prefers JMAP role, but // falls back to name matching ("trash" / "deleted") so users with custom or // pre-existing folders (e.g. "Deleted Items") aren't silently destroyed. @@ -685,6 +754,7 @@ export const useEmailStore = create((set, get) => ({ unifiedErrors: new Map(), unifiedCounts: [], crossUnreadCount: 0, + unifiedScope: [], // Scheduled send state scheduledEmails: [], @@ -2522,30 +2592,37 @@ export const useEmailStore = create((set, get) => ({ // Get the current account ID from the client (assuming primary account) const accountId = client.getAccountId(); - // Check if there are changes for this account + // Changes may arrive for the client's primary account OR a delegated + // shared/group owner it has access to. Active-account *view* concerns + // (current email list, scheduled, calendar, filters) key off the primary + // account only, but the mailbox-COUNT refresh must react to any changed + // account: the active client's getAllMailboxes returns own + delegated + // folders, and the unified-section counts project from that list. (#281) const accountChanges = change.changed[accountId]; - if (!accountChanges) return; + const anyMailboxChanged = Object.values(change.changed).some((c) => c?.Mailbox); // Handle Email state changes - refresh current mailbox - if (accountChanges.Email) { + if (accountChanges?.Email) { await get().refreshCurrentMailbox(client); get().fetchTagCounts(client); } - if (accountChanges.EmailSubmission) { + if (accountChanges?.EmailSubmission) { await get().refreshScheduledMetadata(client); if (get().isScheduledView) { await get().fetchScheduledEmails(client); } } - // Handle Mailbox state changes - refresh mailbox list - if (accountChanges.Mailbox) { + // Handle Mailbox state changes - refresh mailbox list (own + delegated + // shared folders), so both the active account's and its shared folders' + // counters follow background activity. + if (anyMailboxChanged) { await get().fetchMailboxes(client); } // Handle Calendar/CalendarEvent state changes - refresh calendar data - if (accountChanges.Calendar || accountChanges.CalendarEvent) { + if (accountChanges?.Calendar || accountChanges?.CalendarEvent) { const calendarStore = useCalendarStore.getState(); if (calendarStore.supportsCalendar) { calendarStore.fetchCalendars(client); @@ -2563,7 +2640,7 @@ export const useEmailStore = create((set, get) => ({ } // Handle SieveScript state changes - refresh filter rules - if (accountChanges.SieveScript) { + if (accountChanges?.SieveScript) { const { useFilterStore } = await import('./filter-store'); const filterStore = useFilterStore.getState(); if (filterStore.isSupported) { @@ -3183,8 +3260,13 @@ export const useEmailStore = create((set, get) => ({ refreshUnifiedCounts: async (accounts) => { try { - const counts = fetchUnifiedMailboxCounts(accounts); - set({ unifiedCounts: counts }); + // Store the scope and project live counters over it. The badges then track + // the live mailbox lists via the store subscription (below), so subsequent + // optimistic mutations update them without another build/fetch. + set((state) => { + const next = { ...state, unifiedScope: accounts }; + return { unifiedScope: accounts, ...projectUnifiedCounts(next) }; + }); } catch (error) { console.error('Failed to refresh unified counts:', error); } @@ -3223,7 +3305,10 @@ export const useEmailStore = create((set, get) => ({ refreshCrossCounts: (accounts) => { try { - set({ crossUnreadCount: getCrossUnreadTotal(accounts) }); + set((state) => { + const next = { ...state, unifiedScope: accounts }; + return { unifiedScope: accounts, ...projectUnifiedCounts(next) }; + }); } catch (error) { console.error('Failed to refresh cross-account counts:', error); } @@ -3623,3 +3708,31 @@ export const useEmailStore = create((set, get) => ({ }); }, })); + +// Keep the unified-section badges in lockstep with the live per-account mailbox +// lists. Whenever `mailboxes`, `accountMailboxes`, or the unified scope change - +// i.e. after any optimistic delete/move/markRead patch or a push-driven mailbox +// refresh - re-project the badges from that single source of truth. The guard +// short-circuits on every unrelated state change (emails, loading flags, …) by +// reference equality, and our own counter writes don't re-enter the projection +// because they leave the three watched lists untouched (no loop). (#281) +useEmailStore.subscribe((state, prev) => { + if ( + state.mailboxes === prev.mailboxes && + state.accountMailboxes === prev.accountMailboxes && + state.unifiedScope === prev.unifiedScope + ) { + return; + } + if (state.unifiedScope.length === 0) return; + const projected = projectUnifiedCounts(state); + const sameCross = projected.crossUnreadCount === state.crossUnreadCount; + const sameUnified = + projected.unifiedCounts.length === state.unifiedCounts.length && + projected.unifiedCounts.every((c, i) => { + const cur = state.unifiedCounts[i]; + return cur && cur.role === c.role && cur.unreadEmails === c.unreadEmails && cur.totalEmails === c.totalEmails; + }); + if (sameCross && sameUnified) return; + useEmailStore.setState(projected); +});