fix(unified-mailbox): single-source unified counters, unified id space, background push

The unified-section sidebar badges (per-role unified folders + cross-view
All mail/Unread/Starred) failed to count down when messages were deleted/moved/
read from the unified views, and failed to count up for incoming mail - while
the underlying per-account folder counters updated correctly. Root cause: the
badges were a separate counter representation, recomputed only by a fresh server
fetch, completely decoupled from the optimistically-patched mailbox lists.

Three coordinated changes:

V1 - single source of truth: derive `unifiedCounts`/`crossUnreadCount` as a pure
live projection of `mailboxes` + `accountMailboxes` (the lists every mutation
already patches and push refreshes), over the last-known unified scope. A store
subscription re-projects whenever those lists change, so optimistic deletes and
push refreshes flow into the badges with no server round trip and no
eventual-consistency snap-back.

V3 - unified id space: searchEmails/advancedSearchEmails now namespace shared/
delegated mailboxIds (`${ownerId}:${id}`) like getEmails already did. The
cross-account views browse via advancedSearchEmails, so shared emails there
previously carried bare owner ids; now every fetch path is consistent and
emailInMailbox hits the `ids[mailbox.id]` fast path (originalId branches kept as
a defensive fallback). resolveSourceFolderName matches `m.id` first (also fixes
a latent missing source-folder name for shared emails).

Background push: bind push notifications for every connected login, not just the
active one - background accounts now drive the unified counters by rebuilding the
unified scope on their state changes. handleStateChange also refreshes the
mailbox list on a Mailbox change for ANY changed account key, so delegated
shared-folder activity arriving via the active client updates counters too.

Tests: unified-badge live projection on delete; client-level namespacing for
searchEmails/advancedSearchEmails (shared vs own account).
This commit is contained in:
Stefan Hildebrandt
2026-07-11 21:14:50 +02:00
parent fdad60cf03
commit 2e42693228
6 changed files with 300 additions and 36 deletions
+35 -12
View File
@@ -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;
const clients = useAuthStore.getState().getAllConnectedClients();
const cleanups: Array<() => void> = [];
for (const [accId, c] of clients) {
try {
client.onStateChange((change) => handleStateChange(change, client));
const pushEnabled = client.setupPushNotifications();
if (pushEnabled) {
setPushConnected(true);
debug.log('push', '[Push] Push notifications successfully enabled');
if (accId === activeAccountId) {
c.onStateChange((change) => handleStateChange(change, c));
} else {
debug.log('push', '[Push] Push notifications not available on this server');
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:', error);
debug.log('push', '[Push] Failed to setup push notifications for account:', accId, 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
@@ -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<string, unknown>) {
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 });
});
});
});
+14
View File
@@ -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);
+6 -1
View File
@@ -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;
}
@@ -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,
+133 -20
View File
@@ -82,9 +82,23 @@ interface EmailStore {
// isUnifiedView.
crossView: CrossView | null;
unifiedErrors: Map<string, string>; // 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<string, boolean>; 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<string, Mailbox[]> },
): 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<string, Mailbox[]> },
): 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<string, Mailbox[]> },
): { 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<EmailStore>((set, get) => ({
unifiedErrors: new Map(),
unifiedCounts: [],
crossUnreadCount: 0,
unifiedScope: [],
// Scheduled send state
scheduledEmails: [],
@@ -2522,30 +2592,37 @@ export const useEmailStore = create<EmailStore>((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<EmailStore>((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<EmailStore>((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<EmailStore>((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<EmailStore>((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);
});