fix: open right conversations in shared mailbox #814

components/mail/mail-app.tsx

@@ -35,6 +35,7 @@ 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 { resolveThreadRoute } from "@/lib/thread-routing";
 import {
   ErrorBoundary,
   SidebarErrorFallback,
@@ -366,6 +367,14 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
     refreshCurrentMailbox,
   } = useEmailStore();
 
+  // Mailboxes of the account currently being browsed. Mirrors the store's
+  // action-mailbox resolution so a lookup by `selectedMailbox` hits the same
+  // list the email fetch used.
+  const viewMailboxes = useMemo(
+    () => (viewingAccountId ? (accountMailboxes[viewingAccountId] ?? mailboxes) : mailboxes),
+    [viewingAccountId, accountMailboxes, mailboxes],
+  );
+
   // Load recent recipients (from the Sent folder) for compose autocomplete.
   // Runs once when the Sent mailbox is known; the store guards against reloads.
   useEffect(() => {
@@ -1493,14 +1502,20 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
           const repliedEmail = emailState.emails.find(e => e.id === originalEmailId);
           if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) {
             // Route to the email's own account so shared/group threads refresh
-            // from the right server, not the active one. (#281)
-            const threadClient = emailState.isUnifiedView && repliedEmail.sourceClientAccountId
-              ? (useAuthStore.getState().getClientForAccount(repliedEmail.sourceClientAccountId) ?? client)
+            // from the right server, not the active one. (#281, #814)
+            const route = resolveThreadRoute({
+              isUnifiedView: emailState.isUnifiedView,
+              ref: repliedEmail,
+              mailboxes: viewMailboxes,
+              selectedMailbox: emailState.selectedMailbox,
+            });
+            const threadClient = route.clientAccountId
+              ? (useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? client)
               : client;
-            const accountId = emailState.isUnifiedView && repliedEmail.sourceAccountId
-              ? repliedEmail.sourceAccountId
-              : client.getAccountId();
-            const fullEmails = await threadClient.getThreadEmails(repliedEmail.threadId, accountId);
+            const fullEmails = await threadClient.getThreadEmails(
+              repliedEmail.threadId,
+              route.accountId ?? client.getAccountId(),
+            );
             if (fullEmails.length > 0) {
               useEmailStore.setState((state) => {
                 const c = new Map(state.threadEmailsCache);
@@ -2858,14 +2873,20 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
     const repliedEmail = emailState.emails.find(e => e.id === originalEmailId);
     if (repliedEmail?.threadId && emailState.expandedThreadIds.has(repliedEmail.threadId)) {
       // Route to the email's own account so shared/group threads refresh from
-      // the right server, not the active one. (#281)
-      const threadClient = emailState.isUnifiedView && repliedEmail.sourceClientAccountId
-        ? (useAuthStore.getState().getClientForAccount(repliedEmail.sourceClientAccountId) ?? client)
+      // the right server, not the active one. (#281, #814)
+      const route = resolveThreadRoute({
+        isUnifiedView: emailState.isUnifiedView,
+        ref: repliedEmail,
+        mailboxes: viewMailboxes,
+        selectedMailbox: emailState.selectedMailbox,
+      });
+      const threadClient = route.clientAccountId
+        ? (useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? client)
         : client;
-      const accountId = emailState.isUnifiedView && repliedEmail.sourceAccountId
-        ? repliedEmail.sourceAccountId
-        : client.getAccountId();
-      const fullEmails = await threadClient.getThreadEmails(repliedEmail.threadId, accountId);
+      const fullEmails = await threadClient.getThreadEmails(
+        repliedEmail.threadId,
+        route.accountId ?? client.getAccountId(),
+      );
       if (fullEmails.length > 0) {
         useEmailStore.setState((state) => {
           const c = new Map(state.threadEmailsCache);
@@ -3017,17 +3038,17 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
     setActiveView("viewer");
 
     try {
-      // In unified/aggregate views the thread may belong to another (possibly
-      // shared/group) account. Route the fetch to the login it's reachable
-      // through (`sourceClientAccountId`) and pass its owning JMAP account
-      // (`sourceAccountId`) so the thread loads from the right server instead of
-      // the active one (which doesn't have it → empty/body-less). (#281)
+      // The thread may belong to another account: in unified/aggregate views via
+      // the email's own source stamp (#281), and in a directly-browsed shared
+      // folder via the selected mailbox's owner (#814). Either way the fetch has
+      // to name that account - thread ids are per-account, so an unscoped fetch
+      // resolves to an unrelated thread in the active one.
       const ref = thread.emails?.[0];
-      const threadClient = isUnifiedView && ref?.sourceClientAccountId
-        ? (useAuthStore.getState().getClientForAccount(ref.sourceClientAccountId) ?? client)
+      const route = resolveThreadRoute({ isUnifiedView, ref, mailboxes: viewMailboxes, selectedMailbox });
+      const threadClient = route.clientAccountId
+        ? (useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? client)
         : client;
-      const threadAccountId = isUnifiedView ? ref?.sourceAccountId : undefined;
-      const emails = await threadClient.getThreadEmails(thread.threadId, threadAccountId);
+      const emails = await threadClient.getThreadEmails(thread.threadId, route.accountId);
       // Re-stamp the source reference so conversation actions (reply/move/…)
       // resolve to the right account; the fetched objects don't carry it.
       if (isUnifiedView && ref) {

lib/__tests__/thread-routing.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect } from 'vitest';
+import { resolveThreadRoute } from '../thread-routing';
+import { makeEmail, makeMailbox } from './helpers/factories';
+
+const ownInbox = makeMailbox({ id: 'inbox', role: 'inbox' });
+const sharedInbox = makeMailbox({
+  id: 'shared-acct-b__inbox',
+  originalId: 'inbox',
+  role: 'inbox',
+  isShared: true,
+  accountId: 'acct-b',
+});
+const mailboxes = [ownInbox, sharedInbox];
+
+describe('resolveThreadRoute', () => {
+  it('leaves the fetch unscoped for a folder in the user\'s own account', () => {
+    expect(
+      resolveThreadRoute({ isUnifiedView: false, ref: makeEmail(), mailboxes, selectedMailbox: 'inbox' })
+    ).toEqual({ accountId: undefined });
+  });
+
+  // #814: thread ids are per-account, so a shared folder's thread id names a
+  // different conversation in the user's own account. Without the owner the
+  // fetch silently returns that unrelated thread.
+  it('scopes the fetch to the owner when a shared folder is browsed directly', () => {
+    expect(
+      resolveThreadRoute({
+        isUnifiedView: false,
+        // Directly-browsed shared emails carry no source stamp.
+        ref: makeEmail({ sourceClientAccountId: undefined, sourceAccountId: undefined }),
+        mailboxes,
+        selectedMailbox: 'shared-acct-b__inbox',
+      })
+    ).toEqual({ accountId: 'acct-b' });
+  });
+
+  it('prefers the email\'s own source stamp in aggregate views', () => {
+    expect(
+      resolveThreadRoute({
+        isUnifiedView: true,
+        ref: makeEmail({ sourceClientAccountId: 'login-a', sourceAccountId: 'acct-c' }),
+        mailboxes,
+        // Virtual in unified views, so it must not drive the routing.
+        selectedMailbox: '__unified_inbox__',
+      })
+    ).toEqual({ clientAccountId: 'login-a', accountId: 'acct-c' });
+  });
+
+  it('falls back to the selected mailbox when an aggregate email is unstamped', () => {
+    expect(
+      resolveThreadRoute({
+        isUnifiedView: true,
+        ref: makeEmail({ sourceClientAccountId: 'login-a' }),
+        mailboxes,
+        selectedMailbox: 'shared-acct-b__inbox',
+      })
+    ).toEqual({ accountId: 'acct-b' });
+  });
+
+  it('tolerates a missing ref and an unknown mailbox', () => {
+    expect(resolveThreadRoute({ isUnifiedView: false, mailboxes, selectedMailbox: null }))
+      .toEqual({ accountId: undefined });
+    expect(resolveThreadRoute({ isUnifiedView: true, ref: null, mailboxes, selectedMailbox: 'gone' }))
+      .toEqual({ accountId: undefined });
+  });
+});

lib/thread-routing.ts

@@ -0,0 +1,59 @@
+// Account routing for thread fetches (Thread/get + Email/get).
+//
+// JMAP thread ids are scoped to an account, so the same id names a *different*
+// conversation - or none at all - in any other account. A thread fetch that
+// doesn't name the owning account is answered by the active account, which
+// happily returns whatever thread carries that id there: an unrelated
+// conversation from the user's own mailbox. (#814)
+
+import type { Email, Mailbox } from '@/lib/jmap/types';
+
+export interface ThreadRoute {
+  /**
+   * AccountEntry.id of the login the thread is reachable through, or undefined
+   * when the active client already is that login.
+   */
+  clientAccountId?: string;
+  /**
+   * Owning JMAP account to scope the fetch to, or undefined for the client's
+   * own primary account.
+   */
+  accountId?: string;
+}
+
+/**
+ * Resolves which login and which JMAP account a thread must be fetched from.
+ *
+ * Two sources of truth, in order:
+ *  - Aggregate views (unified / cross-account) stamp every email with its own
+ *    `sourceClientAccountId` + `sourceAccountId`; those are unambiguous across
+ *    personal and shared/group sources, so they win.
+ *  - A shared folder browsed directly carries no such stamp - its emails are
+ *    undecorated and reached through the active client - so the owner has to
+ *    come from the selected mailbox instead.
+ *
+ * An empty result means "active client, its own account": the normal
+ * own-mailbox case, and the same request this code sent before shared folders
+ * existed.
+ */
+export function resolveThreadRoute({
+  isUnifiedView,
+  ref,
+  mailboxes,
+  selectedMailbox,
+}: {
+  isUnifiedView: boolean;
+  /** Any email known to belong to the thread, used for its source stamp. */
+  ref?: Pick<Email, 'sourceClientAccountId' | 'sourceAccountId'> | null;
+  mailboxes: Mailbox[];
+  selectedMailbox?: string | null;
+}): ThreadRoute {
+  if (isUnifiedView && ref?.sourceClientAccountId && ref?.sourceAccountId) {
+    return { clientAccountId: ref.sourceClientAccountId, accountId: ref.sourceAccountId };
+  }
+
+  const mailbox = selectedMailbox
+    ? mailboxes.find((mb) => mb.id === selectedMailbox)
+    : undefined;
+  return { accountId: mailbox?.isShared ? mailbox.accountId : undefined };
+}

stores/email-store.ts

@@ -6,6 +6,7 @@ import { useSettingsStore } from "@/stores/settings-store";
 import { useCalendarStore } from "@/stores/calendar-store";
 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 type { ExternalSearchResult } from "@/lib/plugin-types";
 import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, getCrossUnreadTotal, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox";
 import { useAuthStore } from "@/stores/auth-store";
@@ -3338,18 +3339,19 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
       // 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);
+      const route = resolveThreadRoute({
+        isUnifiedView: get().isUnifiedView,
+        ref: threadEmail,
+        mailboxes,
+        selectedMailbox,
+      });
       let actionClient = resolveActionClient(client);
-      let accountId: string | undefined;
-      if (get().isUnifiedView && threadEmail?.sourceClientAccountId && threadEmail?.sourceAccountId) {
-        actionClient = useAuthStore.getState().getClientForAccount(threadEmail.sourceClientAccountId) ?? actionClient;
-        accountId = threadEmail.sourceAccountId;
-      } else {
-        const mailbox = mailboxes.find(mb => mb.id === selectedMailbox);
-        accountId = mailbox?.isShared ? mailbox.accountId : undefined;
+      if (route.clientAccountId) {
+        actionClient = useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? actionClient;
       }
 
       // Fetch all emails in the thread
-      const emails = await actionClient.getThreadEmails(threadId, accountId);
+      const emails = await actionClient.getThreadEmails(threadId, route.accountId);
 
       // Re-stamp the source reference so actions on thread emails resolve to the
       // right account (the fetched objects don't carry it).