From 3548134b96d294a92bdf3fe178784127a86559cc Mon Sep 17 00:00:00 2001 From: bartfaizoli76 Date: Sat, 5 Sep 2026 16:15:44 +0200 Subject: [PATCH 1/2] fix: sandbox locale ignores the app's own 'Automatic' language resolution setSandboxLocale() (which drives api.i18n.locale for every plugin) was fed the RAW useLocaleStore value, which defaults to '' and stays '' under 'Automatic' language (the language-switcher's default option - see components/ui/language-switcher.tsx, components/settings/ language-settings.tsx). setSandboxLocale() then silently no-ops on an empty string, leaving plugins stuck on the hardcoded 'en' default even when the app's own native UI is fully localized via browser-language auto-detection (document.documentElement.lang / detectBrowserLocale, see i18n/detect-locale.ts's getEffectiveLocale() - already the exact function the app's own IntlProvider uses to resolve this same case). Switched both setSandboxLocale() call sites in plugin-store.ts to use getEffectiveLocale() instead of the raw store value, so a plugin's api.i18n.locale now matches the user's ACTUAL visible language, 'Automatic' included, not just an explicit language-switcher choice. Found while building the webdav-storage plugin's localization: native UI showed Hungarian, but the plugin's own slotApi.i18n.locale reported 'en' - confirmed via the plugin's own debug logging, then traced from there rather than guessed. --- stores/plugin-store.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/stores/plugin-store.ts b/stores/plugin-store.ts index 2e98df1d5..3074a8e75 100644 --- a/stores/plugin-store.ts +++ b/stores/plugin-store.ts @@ -8,6 +8,7 @@ import { pluginStorage } from '@/lib/plugin-storage'; import { extractPlugin } from '@/lib/plugin-validator'; import { loadPlugin, deactivatePlugin, setPluginStoreAccessor, setupAutoDisable, setSandboxLocale } from '@/lib/plugin-loader'; import { useLocaleStore } from '@/stores/locale-store'; +import { getEffectiveLocale } from '@/i18n/detect-locale'; import { removeAllPluginHooks } from '@/lib/plugin-hooks'; import { requestConsent } from '@/lib/plugin-sandbox/consent'; import { sha256Hex } from '@/lib/plugin-sandbox/bundle-integrity'; @@ -268,10 +269,20 @@ export const usePluginStore = create()( // in the dead activateAllPlugins() path, so the sandbox locale stayed // 'en' forever and plugin i18n never localized). Subscribe once for // later language switches; those affect plugins/slots loaded after. - setSandboxLocale(useLocaleStore.getState().locale); + // + // Use getEffectiveLocale(), NOT the raw store value: the stored + // choice is '' by default and stays '' under "Automatic" (see + // language-switcher.tsx / settings/language-settings.tsx) - the + // app's own UI resolves that via document.documentElement.lang / + // detectBrowserLocale (see i18n/detect-locale.ts), so a plugin + // reading the raw store value would see an empty string (falling + // back to English) even when the app itself is fully localized to + // the visitor's browser language. Found via a plugin dev report: + // native UI in Hungarian, plugin's own i18n stuck on English. + setSandboxLocale(getEffectiveLocale()); if (!localeSubscribed) { localeSubscribed = true; - useLocaleStore.subscribe((s) => setSandboxLocale(s.locale)); + useLocaleStore.subscribe(() => setSandboxLocale(getEffectiveLocale())); } // Sync server-managed plugins before loading From 5b63993bfe7d6d314171c000920c91da83793c29 Mon Sep 17 00:00:00 2001 From: lucamzanon Date: Thu, 10 Sep 2026 23:03:27 +0200 Subject: [PATCH 2/2] fix: scope thread identity to the owning account in unified views #1012 JMAP thread ids are only unique within their account - Stalwart hands out per-account counters such as "b" - so the cross-account views merged unrelated conversations that happened to share an id, and every client-side thread map (expansion, cache, loading flag, counts, virtualizer key) mixed them up the same way. Give ThreadGroup an account-scoped `threadKey` derived from the email's source stamps and key grouping and all thread state by it. `threadId` stays the raw id for Thread/get, which #281 already routes to the owning account. fetchThreadEmailCounts now asks each source account for its own threads instead of one client for every id. Single-account views carry no stamps, so their key is the bare id and nothing changes there. --- components/email/email-list.tsx | 26 +-- components/mail/mail-app.tsx | 24 +-- components/pro/pro-compose-tab-body.tsx | 5 +- lib/__tests__/thread-utils.test.ts | 59 +++++++ lib/jmap/types.ts | 3 +- lib/thread-utils.ts | 50 ++++-- .../__tests__/email-store-thread-key.test.ts | 151 ++++++++++++++++++ stores/email-store.ts | 95 +++++++---- 8 files changed, 341 insertions(+), 72 deletions(-) create mode 100644 stores/__tests__/email-store-thread-key.test.ts diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 9ca9b8294..71ef9e256 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -13,7 +13,7 @@ import { useEmailStore } from "@/stores/email-store"; import { useAuthStore } from "@/stores/auth-store"; import { useSettingsStore } from "@/stores/settings-store"; import { useUIStore } from "@/stores/ui-store"; -import { groupEmailsByThread, sortThreadGroups } from "@/lib/thread-utils"; +import { groupEmailsByThread, sortThreadGroups, threadKeyFor } from "@/lib/thread-utils"; import { useContextMenu } from "@/hooks/use-context-menu"; import { useConfirmDialog } from "@/hooks/use-confirm-dialog"; import { useTranslations } from "next-intl"; @@ -204,7 +204,7 @@ export function EmailList({ getScrollElement: () => parentRef.current, estimateSize, overscan: 5, - getItemKey: (index) => threadGroups[index]?.threadId ?? String(index), + getItemKey: (index) => threadGroups[index]?.threadKey ?? String(index), }); const LoadingSkeleton = () => ( @@ -323,16 +323,16 @@ export function EmailList({ } }, [client, hasMoreEmails, isLoadingMore, isLoading, isScheduledView, loadMoreEmails, onLoadMoreScheduled]); - const handleToggleThreadExpansion = useCallback(async (threadId: string) => { - const isExpanded = expandedThreadIds.has(threadId); + const handleToggleThreadExpansion = useCallback(async (threadKey: string) => { + const isExpanded = expandedThreadIds.has(threadKey); if (!isExpanded && client) { - toggleThreadExpansion(threadId); - await fetchThreadEmails(client, threadId); + toggleThreadExpansion(threadKey); + await fetchThreadEmails(client, threadKey); // Mark all unread emails in this thread as read - void markThreadAsRead(client, threadId); + void markThreadAsRead(client, threadKey); } else { - toggleThreadExpansion(threadId); + toggleThreadExpansion(threadKey); } }, [client, expandedThreadIds, toggleThreadExpansion, fetchThreadEmails, markThreadAsRead]); @@ -567,18 +567,18 @@ export function EmailList({ > handleToggleThreadExpansion(thread.threadId)} + isLoading={isLoadingThread === thread.threadKey} + expandedEmails={threadEmailsCache.get(thread.threadKey)} + onToggleExpand={() => handleToggleThreadExpansion(thread.threadKey)} onCollapseAllThreads={collapseAllThreads} onEmailSelect={(email) => { // Collapse expanded threads when selecting an email outside the expanded thread const currentExpanded = useEmailStore.getState().expandedThreadIds; if (currentExpanded.size > 0) { // Check if the selected email belongs to any expanded thread via its threadId - if (!email.threadId || !currentExpanded.has(email.threadId)) { + if (!email.threadId || !currentExpanded.has(threadKeyFor(email))) { collapseAllThreads(); } } diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 63d6339b0..dc821d1d6 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -53,7 +53,7 @@ import { debug } from "@/lib/debug"; import { playNotificationSound } from "@/lib/notification-sound"; import { cn } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; -import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, groupEmailsByThread } from "@/lib/thread-utils"; +import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, groupEmailsByThread, threadKeyFor } from "@/lib/thread-utils"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { ErrorBoundary, @@ -830,14 +830,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // thread's messages and marks them read, collapsing only toggles. (#683) onToggleThreadExpansion: () => { if (isScheduledView || !client) return; - const threadId = selectedEmail?.threadId; - if (!threadId) return; + if (!selectedEmail?.threadId) return; + const threadKey = threadKeyFor(selectedEmail); const store = useEmailStore.getState(); - const wasExpanded = store.expandedThreadIds.has(threadId); - store.toggleThreadExpansion(threadId); + const wasExpanded = store.expandedThreadIds.has(threadKey); + store.toggleThreadExpansion(threadKey); if (!wasExpanded) { - void store.fetchThreadEmails(client, threadId).then(() => { - void useEmailStore.getState().markThreadAsRead(client, threadId); + void store.fetchThreadEmails(client, threadKey).then(() => { + void useEmailStore.getState().markThreadAsRead(client, threadKey); }); } }, @@ -1745,7 +1745,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (originalEmailId) { const emailState = useEmailStore.getState(); const repliedEmail = emailState.emails.find(e => e.id === originalEmailId); - if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(threadKeyFor(repliedEmail))) { // Route to the email's own account so shared/group threads refresh // from the right server, not the active one. (#281, #814) const route = resolveThreadRoute({ @@ -1764,7 +1764,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (fullEmails.length > 0) { useEmailStore.setState((state) => { const c = new Map(state.threadEmailsCache); - c.set(repliedEmail.threadId!, fullEmails); + c.set(threadKeyFor(repliedEmail), fullEmails); return { threadEmailsCache: c }; }); } @@ -2280,7 +2280,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } } - if (conversationThread?.threadId === emailToArchive.threadId) { + if (conversationThread && conversationThread.threadKey === threadKeyFor(emailToArchive)) { setConversationThread(null); setConversationEmails([]); } @@ -3201,7 +3201,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // view shows the newly sent reply without collapsing. const emailState = useEmailStore.getState(); const repliedEmail = emailState.emails.find(e => e.id === originalEmailId); - if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(threadKeyFor(repliedEmail))) { // Route to the email's own account so shared/group threads refresh from // the right server, not the active one. (#281, #814) const route = resolveThreadRoute({ @@ -3220,7 +3220,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (fullEmails.length > 0) { useEmailStore.setState((state) => { const c = new Map(state.threadEmailsCache); - c.set(repliedEmail.threadId!, fullEmails); + c.set(threadKeyFor(repliedEmail), fullEmails); return { threadEmailsCache: c }; }); } diff --git a/components/pro/pro-compose-tab-body.tsx b/components/pro/pro-compose-tab-body.tsx index efd6413f8..f7d83a926 100644 --- a/components/pro/pro-compose-tab-body.tsx +++ b/components/pro/pro-compose-tab-body.tsx @@ -11,6 +11,7 @@ import { resolveComposeAccountEmail } from "@/lib/reply-identity"; import { toast } from "@/stores/toast-store"; import { useProTabStore, registerProTabCloseInterceptor, type ProComposeTabData } from "@/stores/pro-tab-store"; import { debug } from "@/lib/debug"; +import { threadKeyFor } from "@/lib/thread-utils"; interface ProComposeTabBodyProps { tabId: string; @@ -118,13 +119,13 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { if (data.sourceEmailId) { const emailState = useEmailStore.getState(); const repliedEmail = emailState.emails.find(e => e.id === data.sourceEmailId); - if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) { + if (repliedEmail?.threadId && emailState.expandedThreadIds.has(threadKeyFor(repliedEmail))) { const accountId = client.getAccountId(); const fullEmails = await client.getThreadEmails(repliedEmail.threadId, accountId); if (fullEmails.length > 0) { useEmailStore.setState((state) => { const c = new Map(state.threadEmailsCache); - c.set(repliedEmail.threadId!, fullEmails); + c.set(threadKeyFor(repliedEmail), fullEmails); return { threadEmailsCache: c }; }); } diff --git a/lib/__tests__/thread-utils.test.ts b/lib/__tests__/thread-utils.test.ts index 8d63c7748..095142a7a 100644 --- a/lib/__tests__/thread-utils.test.ts +++ b/lib/__tests__/thread-utils.test.ts @@ -8,6 +8,8 @@ import { getEmailTagIds, getThreadTagId, getThreadTagIds, + threadKeyFor, + threadIdFromKey, } from '../thread-utils'; import type { Email, ThreadGroup } from '../jmap/types'; @@ -120,6 +122,7 @@ describe('groupEmailsByThread', () => { describe('sortThreadGroups', () => { const makeGroup = (threadId: string, receivedAt: string, hasPinned = false): ThreadGroup => ({ threadId, + threadKey: threadId, emails: [makeEmail({ receivedAt })], latestEmail: makeEmail({ receivedAt }), participantNames: ['A'], @@ -222,6 +225,7 @@ describe('mergeThreadEmails', () => { it('merges new emails without duplicating existing ones', () => { const existing: ThreadGroup = { threadId: 'thread-1', + threadKey: 'thread-1', emails: [ makeEmail({ id: 'e1', receivedAt: '2024-01-10T00:00:00Z' }), makeEmail({ id: 'e2', receivedAt: '2024-01-09T00:00:00Z' }), @@ -248,6 +252,7 @@ describe('mergeThreadEmails', () => { it('updates thread metadata after merge', () => { const existing: ThreadGroup = { threadId: 'thread-1', + threadKey: 'thread-1', emails: [makeEmail({ id: 'e1', keywords: { $seen: true }, hasAttachment: false })], latestEmail: makeEmail({ id: 'e1' }), participantNames: ['Alice'], @@ -403,3 +408,57 @@ describe('getThreadTagIds', () => { expect(getThreadTagIds([])).toEqual([]); }); }); + +describe('cross-account thread identity (#1012)', () => { + // JMAP thread ids are per-account: Stalwart hands out counters like "b", so + // two accounts routinely have a thread "b" that has nothing in common. + const inAccount = (login: string, overrides: Partial = {}): Email => + makeEmail({ sourceClientAccountId: `login-${login}`, sourceAccountId: `acct-${login}`, ...overrides }); + + it('keeps the bare thread id as key for unstamped emails', () => { + expect(threadKeyFor(makeEmail({ threadId: 'b' }))).toBe('b'); + }); + + it('scopes the key by the source stamps', () => { + expect(threadKeyFor(inAccount('a', { threadId: 'b' }))).toBe('login-a/acct-a:b'); + }); + + it('recovers the JMAP id from a key', () => { + expect(threadIdFromKey('login-a/acct-a:b')).toBe('b'); + expect(threadIdFromKey('b')).toBe('b'); + }); + + it('does not merge same-id threads that belong to different accounts', () => { + const groups = groupEmailsByThread([ + inAccount('a', { id: 'a1', threadId: 'b' }), + inAccount('b', { id: 'b1', threadId: 'b' }), + ]); + expect(groups).toHaveLength(2); + // The raw id is preserved for Thread/get on each account … + expect(groups.map((g) => g.threadId)).toEqual(['b', 'b']); + // … while the client-side identity tells them apart. + expect(new Set(groups.map((g) => g.threadKey)).size).toBe(2); + expect(groups.map((g) => g.emails.map((e) => e.id))).toEqual([['a1'], ['b1']]); + }); + + it('still merges the same thread within one account', () => { + const groups = groupEmailsByThread([ + inAccount('a', { id: 'a1', threadId: 'b' }), + inAccount('a', { id: 'a2', threadId: 'b' }), + ]); + expect(groups).toHaveLength(1); + expect(groups[0].emailCount).toBe(2); + }); + + it('looks thread counts up by key', () => { + const counts = new Map([['login-a/acct-a:b', 7]]); + const [group] = groupEmailsByThread([inAccount('a', { threadId: 'b' })], false, counts); + expect(group.emailCount).toBe(7); + }); + + it('uses the message id as both id and key when threading is disabled', () => { + const [group] = groupEmailsByThread([inAccount('a', { id: 'm1', threadId: 'b' })], true); + expect(group.threadKey).toBe('m1'); + expect(group.threadId).toBe('m1'); + }); +}); diff --git a/lib/jmap/types.ts b/lib/jmap/types.ts index 824e2c425..711382c98 100644 --- a/lib/jmap/types.ts +++ b/lib/jmap/types.ts @@ -243,7 +243,8 @@ export interface Thread { // Thread grouping for UI display export interface ThreadGroup { - threadId: string; + threadId: string; // JMAP thread id - unique only within its account; use for Thread/get + threadKey: string; // Client-side identity, unique across accounts (see threadKeyFor) emails: Email[]; // Emails in this thread (sorted by receivedAt desc) latestEmail: Email; // Most recent email participantNames: string[];// Unique participant names diff --git a/lib/thread-utils.ts b/lib/thread-utils.ts index a97b1ffa1..a3476c6ec 100644 --- a/lib/thread-utils.ts +++ b/lib/thread-utils.ts @@ -1,13 +1,41 @@ import type { Email, ThreadGroup } from "./jmap/types"; import { compareEmails, type SortLevel } from "./message-list-order"; +/** + * Client-side identity of a thread. + * + * JMAP thread ids are only unique within their account (Stalwart hands out + * per-account counters such as "b", "c", …), so in views that merge several + * accounts two unrelated conversations can share a `threadId`. Emails fetched + * through the aggregate paths carry their source stamps; scoping the key by + * them keeps those threads apart. Unstamped emails (single-account views) keep + * the bare id, so nothing changes there. `ThreadGroup.threadId` stays the raw + * id for JMAP calls - `threadIdFromKey` is the reverse mapping. + */ +export function threadKeyFor( + email: Pick, +): string { + const scope = [email.sourceClientAccountId, email.sourceAccountId].filter(Boolean).join("/"); + return scope ? `${scope}:${email.threadId}` : email.threadId; +} + +/** + * The JMAP thread id behind a `threadKeyFor` key. JMAP ids never contain ":" + * (RFC 8620 §1.2 limits them to the URL-safe base64 alphabet), so the last + * separator always splits the scope from the id. + */ +export function threadIdFromKey(threadKey: string): string { + const at = threadKey.lastIndexOf(":"); + return at === -1 ? threadKey : threadKey.slice(at + 1); +} + /** * Groups emails by their threadId and creates ThreadGroup objects for UI display. * Single-email threads are still returned as ThreadGroups with emailCount=1. * When disableThreading is true, each email is placed into its own group using * its message ID as the key, so the list shows individual messages. * - * @param threadEmailCounts - Optional map of threadId → total email count across + * @param threadEmailCounts - Optional map of threadKey → total email count across * all folders (from Thread/get). When provided, emailCount reflects the full * thread size rather than just the emails in the current folder. */ @@ -20,21 +48,22 @@ export function groupEmailsByThread( return []; } - // Group emails by threadId (or by message ID when threading is disabled) + // Group by the account-scoped thread key (or by message id when threading + // is disabled): bare thread ids collide across accounts in aggregate views. const threadMap = new Map(); for (const email of emails) { - const threadId = disableThreading ? email.id : email.threadId; - if (!threadMap.has(threadId)) { - threadMap.set(threadId, []); + const key = disableThreading ? email.id : threadKeyFor(email); + if (!threadMap.has(key)) { + threadMap.set(key, []); } - threadMap.get(threadId)!.push(email); + threadMap.get(key)!.push(email); } // Convert to ThreadGroup array const threadGroups: ThreadGroup[] = []; - for (const [threadId, threadEmails] of threadMap) { + for (const [threadKey, threadEmails] of threadMap) { // Sort emails by receivedAt descending (newest first) const sortedEmails = [...threadEmails].sort( (a, b) => new Date(b.receivedAt).getTime() - new Date(a.receivedAt).getTime() @@ -54,7 +83,9 @@ export function groupEmailsByThread( const hasForwarded = sortedEmails.some(e => e.keywords?.$forwarded); threadGroups.push({ - threadId, + // With threading off the "thread" is the single message, keyed by its id. + threadId: disableThreading ? latestEmail.id : latestEmail.threadId, + threadKey, emails: sortedEmails, latestEmail, participantNames, @@ -64,7 +95,7 @@ export function groupEmailsByThread( hasAttachment, hasAnswered, hasForwarded, - emailCount: threadEmailCounts?.get(threadId) ?? sortedEmails.length, + emailCount: threadEmailCounts?.get(threadKey) ?? sortedEmails.length, }); } @@ -161,6 +192,7 @@ export function mergeThreadEmails( return { threadId: existingGroup.threadId, + threadKey: existingGroup.threadKey, emails: mergedEmails, latestEmail, participantNames, diff --git a/stores/__tests__/email-store-thread-key.test.ts b/stores/__tests__/email-store-thread-key.test.ts new file mode 100644 index 000000000..76e6ad336 --- /dev/null +++ b/stores/__tests__/email-store-thread-key.test.ts @@ -0,0 +1,151 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import { useAuthStore } from '../auth-store'; +import { threadKeyFor } from '@/lib/thread-utils'; +import { CROSS_ALL } from '@/lib/jmap/types'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +/** + * #1012: JMAP thread ids are only unique within their account - Stalwart hands + * out per-account counters such as "b" - so in the cross-account views two + * accounts routinely list a thread "b" that has nothing in common. Thread state + * has to be keyed by the account-scoped thread key, and thread fetches routed + * to the account that owns the thread. + */ + +const makeMailbox = (overrides: Partial = {}): Mailbox => ({ + id: 'inbox', + name: 'Inbox', + role: 'inbox', + sortOrder: 0, + totalEmails: 0, + unreadEmails: 0, + totalThreads: 0, + unreadThreads: 0, + myRights: { + mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, + maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true, + }, + isSubscribed: true, + isShared: false, + ...overrides, +}); + +type Login = 'a' | 'b'; + +// Both accounts have a thread "b"; only the source stamps tell them apart. +const inAccount = (login: Login, id: string, overrides: Partial = {}): Email => + ({ + id, + threadId: 'b', + mailboxIds: { inbox: true }, + keywords: {}, + size: 1, + receivedAt: '2026-09-10T10:00:00Z', + from: [{ email: `${login}@example.com` }], + subject: id, + preview: '', + hasAttachment: false, + accountId: `login-${login}`, + accountLabel: `${login}@example.com`, + sourceClientAccountId: `login-${login}`, + sourceAccountId: `acct-${login}`, + ...overrides, + }) as unknown as Email; + +function makeClient(login: Login, threadSize: number) { + return { + getAccountId: () => `acct-${login}`, + getThreads: vi.fn(async (ids: string[]) => + ids.map((id) => ({ id, emailIds: Array.from({ length: threadSize }, (_, i) => `${login}-${id}-${i}`) })), + ), + batchMarkAsRead: vi.fn(async () => {}), + getThreadEmails: vi.fn(async (threadId: string) => [ + inAccount(login, `${login}-full-1`, { threadId, sourceClientAccountId: undefined, sourceAccountId: undefined }), + inAccount(login, `${login}-full-2`, { threadId, sourceClientAccountId: undefined, sourceAccountId: undefined }), + ]), + } as unknown as IJMAPClient & { + getThreads: ReturnType; + getThreadEmails: ReturnType; + batchMarkAsRead: ReturnType; + }; +} + +describe('thread identity across accounts (#1012)', () => { + let clientA: ReturnType; + let clientB: ReturnType; + const a1 = inAccount('a', 'a1'); + const b1 = inAccount('b', 'b1'); + + beforeEach(() => { + clientA = makeClient('a', 3); + clientB = makeClient('b', 1); + useAuthStore.setState({ + activeAccountId: 'login-a', + getClientForAccount: (id: string) => + (id === 'login-a' ? clientA : id === 'login-b' ? clientB : undefined) as never, + } as never); + useEmailStore.setState({ + isUnifiedView: true, + unifiedRole: null, + crossView: 'all', + selectedMailbox: CROSS_ALL, + viewingAccountId: null, + mailboxes: [makeMailbox()], + accountMailboxes: { 'acct-a': [makeMailbox()], 'acct-b': [makeMailbox()] }, + emails: [a1, b1], + expandedThreadIds: new Set(), + threadEmailsCache: new Map(), + threadEmailCounts: new Map(), + isLoadingThread: null, + processingReadStatus: new Set(), + }); + }); + + it('asks each account for its own threads and files the counts under the scoped key', async () => { + await useEmailStore.getState().fetchThreadEmailCounts(clientA); + + expect(clientA.getThreads).toHaveBeenCalledWith(['b'], 'acct-a'); + expect(clientB.getThreads).toHaveBeenCalledWith(['b'], 'acct-b'); + const counts = useEmailStore.getState().threadEmailCounts; + expect(counts.get(threadKeyFor(a1))).toBe(3); + expect(counts.get(threadKeyFor(b1))).toBe(1); + // The bare id must not be a key any more: it could only ever hold one + // account's answer. + expect(counts.has('b')).toBe(false); + }); + + it('fetches a thread from the account that owns it and caches it under the scoped key', async () => { + const emails = await useEmailStore.getState().fetchThreadEmails(clientA, threadKeyFor(b1)); + + expect(clientB.getThreadEmails).toHaveBeenCalledWith('b', 'acct-b'); + expect(clientA.getThreadEmails).not.toHaveBeenCalled(); + expect(emails.map((e) => e.id)).toEqual(['b-full-1', 'b-full-2']); + // Re-stamped with the owning account so later actions route correctly. + expect(emails.every((e) => e.sourceAccountId === 'acct-b' && e.sourceClientAccountId === 'login-b')).toBe(true); + + const cache = useEmailStore.getState().threadEmailsCache; + expect(cache.has(threadKeyFor(b1))).toBe(true); + expect(cache.has(threadKeyFor(a1))).toBe(false); + expect(cache.has('b')).toBe(false); + }); + + it('marks only the owning account\'s messages read when a thread is expanded', async () => { + await useEmailStore.getState().markThreadAsRead(clientA, threadKeyFor(a1)); + + expect(clientA.batchMarkAsRead).toHaveBeenCalledWith(['a1'], true); + expect(clientB.batchMarkAsRead).not.toHaveBeenCalled(); + const byId = new Map(useEmailStore.getState().emails.map((e) => [e.id, e])); + expect(byId.get('a1')?.keywords?.$seen).toBe(true); + expect(byId.get('b1')?.keywords?.$seen).toBeUndefined(); + }); + + it('keeps expansion state apart for same-id threads of different accounts', () => { + useEmailStore.getState().toggleThreadExpansion(threadKeyFor(a1)); + + const expanded = useEmailStore.getState().expandedThreadIds; + expect(expanded.has(threadKeyFor(a1))).toBe(true); + expect(expanded.has(threadKeyFor(b1))).toBe(false); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index ac048655c..c10ac3a1a 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -8,6 +8,7 @@ import type { SortLevel } from "@/lib/message-list-order"; import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty } from "@/lib/jmap/search-utils"; import { emailHooks } from "@/lib/plugin-hooks"; import { resolveThreadRoute } from "@/lib/thread-routing"; +import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils"; import type { ExternalSearchResult } from "@/lib/plugin-types"; import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, getCrossUnreadTotal, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; import { useAuthStore } from "@/stores/auth-store"; @@ -91,6 +92,9 @@ interface EmailStore { newEmailNotification: Email | null; // New email notification for toast // Thread expansion state + // Thread state is keyed by the account-scoped thread key (threadKeyFor), not + // the bare JMAP id: in aggregate views the same id names unrelated threads + // in different accounts. Same for threadEmailCounts below. expandedThreadIds: Set; threadEmailsCache: Map; isLoadingThread: string | null; @@ -289,12 +293,12 @@ interface EmailStore { clearNewEmailNotification: () => void; // Thread expansion actions - toggleThreadExpansion: (threadId: string) => void; - fetchThreadEmails: (client: IJMAPClient, threadId: string) => Promise; + toggleThreadExpansion: (threadKey: string) => void; + fetchThreadEmails: (client: IJMAPClient, threadKey: string) => Promise; collapseAllThreads: () => void; updateThreadCache: (threadId: string, emails: Email[]) => void; fetchThreadEmailCounts: (client: IJMAPClient) => Promise; - markThreadAsRead: (client: IJMAPClient, threadId: string) => Promise; + markThreadAsRead: (client: IJMAPClient, threadKey: string) => Promise; // Mailbox management createMailbox: (client: IJMAPClient, name: string, parentId?: string, accountId?: string) => Promise; @@ -540,9 +544,9 @@ async function applyEmailDeltaNow(client: IJMAPClient, newState: string, get: St .map((e) => replacementById.get(e.id) ?? e); const changedThreadIds = new Set(); for (const e of current) { - if (removed.has(e.id) || replacementById.has(e.id)) changedThreadIds.add(e.threadId); + if (removed.has(e.id) || replacementById.has(e.id)) changedThreadIds.add(threadKeyFor(e)); } - for (const e of replacementById.values()) changedThreadIds.add(e.threadId); + for (const e of replacementById.values()) changedThreadIds.add(threadKeyFor(e)); set((state) => { const threadEmailsCache = new Map(state.threadEmailsCache); @@ -2673,9 +2677,9 @@ export const useEmailStore = create((set, get) => ({ Array.from(currentState.selectedEmailIds).filter(id => !removedEmailIds.has(id)) ); const nextExpandedThreadIds = new Set(currentState.expandedThreadIds); - nextExpandedThreadIds.delete(email.threadId); + nextExpandedThreadIds.delete(threadKeyFor(email)); const nextThreadEmailsCache = new Map(currentState.threadEmailsCache); - nextThreadEmailsCache.delete(email.threadId); + nextThreadEmailsCache.delete(threadKeyFor(email)); return { emails: currentState.emails.filter(currentEmail => !removedEmailIds.has(currentEmail.id)), @@ -3966,8 +3970,8 @@ export const useEmailStore = create((set, get) => ({ // Invalidate thread email caches for threads whose composition changed // so expanded threads pick up new/removed emails. - const prevThreadIds = new Set(currentEmails.map(e => e.threadId)); - const nextThreadIds = new Set(merged.map(e => e.threadId)); + const prevThreadIds = new Set(currentEmails.map(e => threadKeyFor(e))); + const nextThreadIds = new Set(merged.map(e => threadKeyFor(e))); const changedThreadIds = new Set(); for (const tid of prevThreadIds) { if (!nextThreadIds.has(tid)) changedThreadIds.add(tid); @@ -3978,13 +3982,15 @@ export const useEmailStore = create((set, get) => ({ // Also check threads where the set of email IDs changed const prevEmailsByThread = new Map>(); for (const e of currentEmails) { - if (!prevEmailsByThread.has(e.threadId)) prevEmailsByThread.set(e.threadId, new Set()); - prevEmailsByThread.get(e.threadId)!.add(e.id); + const key = threadKeyFor(e); + if (!prevEmailsByThread.has(key)) prevEmailsByThread.set(key, new Set()); + prevEmailsByThread.get(key)!.add(e.id); } const nextEmailsByThread = new Map>(); for (const e of merged) { - if (!nextEmailsByThread.has(e.threadId)) nextEmailsByThread.set(e.threadId, new Set()); - nextEmailsByThread.get(e.threadId)!.add(e.id); + const key = threadKeyFor(e); + if (!nextEmailsByThread.has(key)) nextEmailsByThread.set(key, new Set()); + nextEmailsByThread.get(key)!.add(e.id); } for (const [tid, nextIds] of nextEmailsByThread) { const prevIds = prevEmailsByThread.get(tid); @@ -4050,38 +4056,40 @@ export const useEmailStore = create((set, get) => ({ }, // Thread expansion actions - toggleThreadExpansion: (threadId) => { + toggleThreadExpansion: (threadKey) => { const { expandedThreadIds } = get(); const newExpandedThreadIds = new Set(expandedThreadIds); - if (newExpandedThreadIds.has(threadId)) { - newExpandedThreadIds.delete(threadId); + if (newExpandedThreadIds.has(threadKey)) { + newExpandedThreadIds.delete(threadKey); } else { - newExpandedThreadIds.add(threadId); + newExpandedThreadIds.add(threadKey); } set({ expandedThreadIds: newExpandedThreadIds }); }, - fetchThreadEmails: async (client, threadId) => { + fetchThreadEmails: async (client, threadKey) => { const { threadEmailsCache, selectedMailbox } = get(); const mailboxes = resolveActionMailboxes(); // Check if we already have this thread cached - const cachedEmails = threadEmailsCache.get(threadId); + const cachedEmails = threadEmailsCache.get(threadKey); if (cachedEmails && cachedEmails.length > 0) { return cachedEmails; } // Set loading state - set({ isLoadingThread: threadId }); + set({ isLoadingThread: threadKey }); try { // Route to the thread's own account. In aggregate views `selectedMailbox` // is virtual, so derive the client + accountId from a list email of this // thread (handles shared/group accounts); otherwise fall back to the - // selected-mailbox shared-folder logic. (#281) - const threadEmail = get().emails.find(e => e.threadId === threadId); + // selected-mailbox shared-folder logic. (#281) The key is account-scoped, + // so this can't pick up another account's thread with the same id. + const threadEmail = get().emails.find(e => threadKeyFor(e) === threadKey); + const threadId = threadEmail?.threadId ?? threadIdFromKey(threadKey); const route = resolveThreadRoute({ isUnifiedView: isAggregateListView(), ref: threadEmail, @@ -4109,7 +4117,7 @@ export const useEmailStore = create((set, get) => ({ // Update cache const newCache = new Map(get().threadEmailsCache); - newCache.set(threadId, emails); + newCache.set(threadKey, emails); set({ threadEmailsCache: newCache, @@ -4124,10 +4132,10 @@ export const useEmailStore = create((set, get) => ({ } }, - markThreadAsRead: async (client, threadId) => { + markThreadAsRead: async (client, threadKey) => { const state = get(); - const threadEmails = state.threadEmailsCache.get(threadId) ?? []; - const mainEmails = state.emails.filter(e => e.threadId === threadId); + const threadEmails = state.threadEmailsCache.get(threadKey) ?? []; + const mainEmails = state.emails.filter(e => threadKeyFor(e) === threadKey); // Combine unique emails from both sources const allEmailMap = new Map(); @@ -4175,9 +4183,9 @@ export const useEmailStore = create((set, get) => ({ // Update threadEmailsCache const newCache = new Map(state.threadEmailsCache); - const cached = newCache.get(threadId); + const cached = newCache.get(threadKey); if (cached) { - newCache.set(threadId, cached.map(e => + newCache.set(threadKey, cached.map(e => unreadSet.has(e.id) ? { ...e, keywords: { ...e.keywords, $seen: true } } : e )); } @@ -4224,16 +4232,33 @@ export const useEmailStore = create((set, get) => ({ const { emails } = get(); if (emails.length === 0) return; - const uniqueThreadIds = [...new Set(emails.map(e => e.threadId).filter(Boolean))]; - if (uniqueThreadIds.length === 0) return; + // Thread ids are per-account, so ask each source account for its own + // threads and file the counts under the account-scoped key. One client + // answering for every id would return the ACTIVE account's unrelated + // thread "b" for every other account's thread "b" in an aggregate view. + const bySource = new Map }>(); + for (const e of emails) { + if (!e.threadId) continue; + const source = `${e.sourceClientAccountId ?? ''}/${e.sourceAccountId ?? ''}`; + const entry = bySource.get(source) ?? { ref: e, threadIds: new Set() }; + entry.threadIds.add(e.threadId); + bySource.set(source, entry); + } + if (bySource.size === 0) return; try { - const effectiveClient = resolveActionClient(client); - const threads = await effectiveClient.getThreads(uniqueThreadIds); - const newCounts = new Map(get().threadEmailCounts); - for (const thread of threads) { - newCounts.set(thread.id, thread.emailIds?.length ?? 0); + for (const { ref, threadIds } of bySource.values()) { + const { client: sourceClient, accountId } = resolveEmailActionContext(ref, client); + const threads = await sourceClient.getThreads([...threadIds], accountId); + for (const thread of threads) { + const key = threadKeyFor({ + threadId: thread.id, + sourceClientAccountId: ref.sourceClientAccountId, + sourceAccountId: ref.sourceAccountId, + }); + newCounts.set(key, thread.emailIds?.length ?? 0); + } } set({ threadEmailCounts: newCounts }); } catch {