fix: stop minting duplicate "Trusted Senders" address books #730

lib/__tests__/jmap-contact-client.test.ts

@@ -134,6 +134,31 @@ describe('JMAPClient contact methods', () => {
       const result = await client.getAddressBooks();
       expect(result).toEqual([]);
     });
+
+    it('should throw on network error when throwOnError is set', async () => {
+      const client = createClient();
+      vi.spyOn(globalThis, 'fetch').mockRejectedValue(new Error('Network error'));
+
+      await expect(client.getAddressBooks({ throwOnError: true })).rejects.toThrow('Network error');
+    });
+
+    it('should throw on a JMAP method error when throwOnError is set', async () => {
+      const client = createClient();
+      mockFetch({
+        methodResponses: [['error', { type: 'serverFail', description: 'Too many requests' }, '0']],
+      });
+
+      await expect(client.getAddressBooks({ throwOnError: true })).rejects.toThrow('Too many requests');
+    });
+
+    it('should still resolve to an empty list for a genuinely empty account', async () => {
+      const client = createClient();
+      mockFetch({
+        methodResponses: [['AddressBook/get', { list: [] }, '0']],
+      });
+
+      await expect(client.getAddressBooks({ throwOnError: true })).resolves.toEqual([]);
+    });
   });
 
   describe('getContacts', () => {

lib/jmap/client-interface.ts

@@ -265,12 +265,12 @@ export interface IJMAPClient {
 
   // ── Contacts ──────────────────────────────────────────────────
   getContactsAccountId(): string;
-  getAddressBooks(): Promise<AddressBook[]>;
+  getAddressBooks(options?: { throwOnError?: boolean }): Promise<AddressBook[]>;
   getAllAddressBooks(): Promise<AddressBook[]>;
   createAddressBook(name: string): Promise<AddressBook>;
   updateAddressBook(addressBookId: string, updates: Partial<AddressBook>, targetAccountId?: string): Promise<void>;
   deleteAddressBook(addressBookId: string, targetAccountId?: string): Promise<void>;
-  getContacts(addressBookId?: string): Promise<ContactCard[]>;
+  getContacts(addressBookId?: string, options?: { throwOnError?: boolean }): Promise<ContactCard[]>;
   getAllContacts(): Promise<ContactCard[]>;
   getContact(contactId: string, accountId?: string): Promise<ContactCard | null>;
   createContact(contact: Partial<ContactCard>, targetAccountId?: string): Promise<ContactCard>;

lib/jmap/client.ts

@@ -4241,7 +4241,7 @@ export class JMAPClient implements IJMAPClient {
     return [primaryId, ...accountIds];
   }
 
-  async getAddressBooks(): Promise<AddressBook[]> {
+  async getAddressBooks(options?: { throwOnError?: boolean }): Promise<AddressBook[]> {
     try {
       const accountId = this.getContactsAccountId();
       const response = await this.request([
@@ -4251,9 +4251,13 @@ export class JMAPClient implements IJMAPClient {
       if (response.methodResponses?.[0]?.[0] === "AddressBook/get") {
         return (response.methodResponses[0][1].list || []) as AddressBook[];
       }
-      return [];
+      const methodError = response.methodResponses?.[0]?.[1];
+      throw new Error(methodError?.description || methodError?.type || "AddressBook/get failed");
     } catch (error) {
       console.error('Failed to get address books:', error);
+      // Callers that would treat an empty list as "nothing exists yet" must be
+      // able to tell a real empty account from a failed fetch (#730).
+      if (options?.throwOnError) throw error;
       return [];
     }
   }
@@ -4500,13 +4504,14 @@ export class JMAPClient implements IJMAPClient {
     return allContacts;
   }
 
-  async getContacts(addressBookId?: string): Promise<ContactCard[]> {
+  async getContacts(addressBookId?: string, options?: { throwOnError?: boolean }): Promise<ContactCard[]> {
     try {
       const accountId = this.getContactsAccountId();
       const filter = addressBookId ? { inAddressBook: addressBookId } : undefined;
       return await this.fetchPaginatedContacts(accountId, filter);
     } catch (error) {
       console.error('Failed to get contacts:', error);
+      if (options?.throwOnError) throw error;
       return [];
     }
   }

stores/__tests__/contact-store.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, vi } from 'vitest';
-import { useContactStore, getContactPhotoUri, normalizeContactPhotoUri } from '../contact-store';
+import { useContactStore, getContactPhotoUri, normalizeContactPhotoUri, TRUSTED_SENDERS_BOOK_NAME } from '../contact-store';
 import type { ContactCard } from '@/lib/jmap/types';
 
 vi.stubGlobal('crypto', { randomUUID: () => '00000000-0000-0000-0000-000000000000' });
@@ -560,6 +560,136 @@ describe('contact-store', () => {
     });
   });
 
+  describe('loadTrustedSendersBook (#730)', () => {
+    const trustedBook = (id: string) => ({ id, name: TRUSTED_SENDERS_BOOK_NAME });
+
+    const makeClient = (overrides: Record<string, unknown> = {}) => ({
+      getAddressBooks: vi.fn().mockResolvedValue([]),
+      createAddressBook: vi.fn(async (name: string) => ({ id: 'created-book', name })),
+      getContacts: vi.fn().mockResolvedValue([]),
+      createContact: vi.fn(),
+      updateContact: vi.fn().mockResolvedValue(undefined),
+      deleteContact: vi.fn().mockResolvedValue(undefined),
+      deleteAddressBook: vi.fn().mockResolvedValue(undefined),
+      ...overrides,
+    });
+
+    beforeEach(() => {
+      useContactStore.setState({
+        trustedSendersBookId: null,
+        trustedSendersLoaded: false,
+        trustedSendersLoading: false,
+        trustedSenderEmails: [],
+      });
+    });
+
+    it('does not create a book when the address book fetch fails', async () => {
+      const client = makeClient({ getAddressBooks: vi.fn().mockRejectedValue(new Error('429 rate limited')) });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.createAddressBook).not.toHaveBeenCalled();
+      expect(useContactStore.getState().trustedSendersBookId).toBeNull();
+    });
+
+    it('creates the book exactly once for a genuinely empty account', async () => {
+      const client = makeClient();
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.createAddressBook).toHaveBeenCalledTimes(1);
+      expect(useContactStore.getState().trustedSendersBookId).toBe('created-book');
+    });
+
+    it('shares one in-flight load between concurrent callers', async () => {
+      const client = makeClient();
+
+      await Promise.all([
+        useContactStore.getState().loadTrustedSendersBook(client as never),
+        useContactStore.getState().loadTrustedSendersBook(client as never),
+        useContactStore.getState().loadTrustedSendersBook(client as never),
+      ]);
+
+      expect(client.createAddressBook).toHaveBeenCalledTimes(1);
+    });
+
+    it('picks the same book on every client when duplicates already exist', async () => {
+      const client = makeClient({
+        getAddressBooks: vi.fn().mockResolvedValue([trustedBook('book-c'), trustedBook('book-a'), trustedBook('book-b')]),
+      });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.createAddressBook).not.toHaveBeenCalled();
+      expect(useContactStore.getState().trustedSendersBookId).toBe('book-a');
+    });
+
+    it('merges duplicate books into the canonical one and deletes the leftovers', async () => {
+      const contactsByBook: Record<string, ContactCard[]> = {
+        'book-a': [makeContact({ id: 'c1', addressBookIds: { 'book-a': true }, emails: { e0: { address: 'kept@example.com' } } })],
+        'book-b': [makeContact({ id: 'c2', addressBookIds: { 'book-b': true }, emails: { e0: { address: 'moved@example.com' } } })],
+      };
+      const client = makeClient({
+        getAddressBooks: vi.fn().mockResolvedValue([trustedBook('book-a'), trustedBook('book-b')]),
+        getContacts: vi.fn(async (bookId: string) => contactsByBook[bookId] ?? []),
+      });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.updateContact).toHaveBeenCalledWith('c2', { addressBookIds: { 'book-a': true } });
+      expect(client.deleteAddressBook).toHaveBeenCalledWith('book-b');
+      expect(useContactStore.getState().trustedSenderEmails.sort()).toEqual(['kept@example.com', 'moved@example.com']);
+    });
+
+    it('drops a duplicate card instead of copying it when the address is already trusted', async () => {
+      const duplicate = makeContact({ id: 'c2', addressBookIds: { 'book-b': true }, emails: { e0: { address: 'kept@example.com' } } });
+      const contactsByBook: Record<string, ContactCard[]> = {
+        'book-a': [makeContact({ id: 'c1', addressBookIds: { 'book-a': true }, emails: { e0: { address: 'kept@example.com' } } })],
+        'book-b': [duplicate],
+      };
+      const client = makeClient({
+        getAddressBooks: vi.fn().mockResolvedValue([trustedBook('book-a'), trustedBook('book-b')]),
+        getContacts: vi.fn(async (bookId: string) => contactsByBook[bookId] ?? []),
+      });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.deleteContact).toHaveBeenCalledWith('c2');
+      expect(useContactStore.getState().trustedSenderEmails).toEqual(['kept@example.com']);
+    });
+
+    it('unfiles rather than destroys a card that also lives in another book', async () => {
+      const contactsByBook: Record<string, ContactCard[]> = {
+        'book-a': [makeContact({ id: 'c1', addressBookIds: { 'book-a': true }, emails: { e0: { address: 'kept@example.com' } } })],
+        'book-b': [makeContact({ id: 'c2', addressBookIds: { 'book-b': true, personal: true }, emails: { e0: { address: 'kept@example.com' } } })],
+      };
+      const client = makeClient({
+        getAddressBooks: vi.fn().mockResolvedValue([trustedBook('book-a'), trustedBook('book-b')]),
+        getContacts: vi.fn(async (bookId: string) => contactsByBook[bookId] ?? []),
+      });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.deleteContact).not.toHaveBeenCalled();
+      expect(client.updateContact).toHaveBeenCalledWith('c2', { addressBookIds: { personal: true } });
+    });
+
+    it('keeps a duplicate book when its contacts cannot be read', async () => {
+      const client = makeClient({
+        getAddressBooks: vi.fn().mockResolvedValue([trustedBook('book-a'), trustedBook('book-b')]),
+        getContacts: vi.fn(async (bookId: string) => {
+          if (bookId === 'book-b') throw new Error('Network error');
+          return [];
+        }),
+      });
+
+      await useContactStore.getState().loadTrustedSendersBook(client as never);
+
+      expect(client.deleteAddressBook).not.toHaveBeenCalled();
+      expect(useContactStore.getState().trustedSendersBookId).toBe('book-a');
+    });
+  });
+
   describe('persistence/partialize', () => {
     it('should persist contacts when supportsSync is false', () => {
       const { partialize } = (useContactStore as unknown as { persist: { getOptions: () => { partialize: (state: Record<string, unknown>) => Record<string, unknown> } } }).persist.getOptions();

stores/contact-store.ts

@@ -140,6 +140,69 @@ export function getContactPhotoUri(contact: ContactCard): string | undefined {
 
 export const TRUSTED_SENDERS_BOOK_NAME = 'Trusted Senders';
 
+/** In-flight trusted senders load, shared by all callers to keep it single-flight. */
+let trustedSendersLoadPromise: Promise<void> | null = null;
+
+/** Every e-mail address held by the given contacts, lowercased and de-duplicated. */
+export function collectContactEmails(contacts: ContactCard[]): string[] {
+  const seen = new Set<string>();
+  for (const contact of contacts) {
+    if (!contact.emails) continue;
+    for (const entry of Object.values(contact.emails)) {
+      const address = entry.address?.toLowerCase().trim();
+      if (address) seen.add(address);
+    }
+  }
+  return Array.from(seen);
+}
+
+/**
+ * Fold duplicate "Trusted Senders" books into the canonical one and delete the
+ * emptied leftovers (#730).  Contacts that already exist in the canonical book
+ * are dropped rather than copied, so merging never produces two cards for the
+ * same address.  A duplicate is only destroyed once every one of its contacts
+ * has been moved or removed - if any step fails the book is left untouched so
+ * no trusted sender is lost.
+ */
+export async function consolidateTrustedSenderBooks(
+  client: IJMAPClient,
+  canonicalBookId: string,
+  duplicates: AddressBook[],
+  knownEmails: string[]
+): Promise<string[]> {
+  const emails = new Set(knownEmails);
+
+  for (const duplicate of duplicates) {
+    try {
+      const contacts = await client.getContacts(duplicate.id, { throwOnError: true });
+      for (const contact of contacts) {
+        const addresses = collectContactEmails([contact]);
+        // Cards filed in other books too must never be destroyed - only unfiled
+        // from the duplicate.
+        const otherBookIds = Object.keys(contact.addressBookIds || {}).filter(id => id !== duplicate.id);
+        const addressBookIds: Record<string, boolean> = {};
+        for (const id of otherBookIds) addressBookIds[id] = true;
+
+        if (addresses.some(address => !emails.has(address))) {
+          addressBookIds[canonicalBookId] = true;
+          await client.updateContact(contact.id, { addressBookIds });
+          addresses.forEach(address => emails.add(address));
+        } else if (otherBookIds.length > 0) {
+          await client.updateContact(contact.id, { addressBookIds });
+        } else {
+          await client.deleteContact(contact.id);
+        }
+      }
+      await client.deleteAddressBook(duplicate.id);
+      debug.log('contacts', 'Merged duplicate trusted senders book:', duplicate.id);
+    } catch (error) {
+      debug.error('Failed to merge duplicate trusted senders book:', duplicate.id, error);
+    }
+  }
+
+  return Array.from(emails);
+}
+
 interface ContactStore {
   contacts: ContactCard[];
   addressBooks: AddressBook[];
@@ -1028,28 +1091,48 @@ export const useContactStore = create<ContactStore>()(
       },
 
       loadTrustedSendersBook: async (client) => {
-        if (get().trustedSendersLoading) return;
+        // Share the in-flight load so parallel callers (mail view, settings,
+        // the modal, addToTrustedSendersBook) can never create the book twice.
+        if (trustedSendersLoadPromise) return trustedSendersLoadPromise;
         set({ trustedSendersLoading: true });
-        try {
-          debug.log('contacts', 'Loading trusted senders address book');
-          const books = await client.getAddressBooks();
-          let book = books.find(b => b.name === TRUSTED_SENDERS_BOOK_NAME);
-          if (!book) {
-            debug.log('contacts', 'Creating new trusted senders address book');
-            book = await client.createAddressBook(TRUSTED_SENDERS_BOOK_NAME);
+
+        trustedSendersLoadPromise = (async () => {
+          try {
+            debug.log('contacts', 'Loading trusted senders address book');
+            // Must throw rather than yield [] on failure: treating a failed
+            // fetch as "no book yet" minted a duplicate on every hiccup (#730).
+            const books = await client.getAddressBooks({ throwOnError: true });
+            // Sort so every client picks the same book when duplicates exist.
+            const matches = books
+              .filter(b => b.name === TRUSTED_SENDERS_BOOK_NAME)
+              .sort((a, b) => a.id.localeCompare(b.id));
+
+            let book = matches[0];
+            if (!book) {
+              debug.log('contacts', 'Creating new trusted senders address book');
+              book = await client.createAddressBook(TRUSTED_SENDERS_BOOK_NAME);
+            }
+            const bookId = book.id;
+            debug.log('contacts', 'Trusted senders book id:', bookId);
+            const contacts = await client.getContacts(bookId, { throwOnError: true });
+            debug.log('contacts', 'Loaded', contacts.length, 'trusted sender contacts');
+            let emails = collectContactEmails(contacts);
+
+            if (matches.length > 1) {
+              debug.log('contacts', 'Found', matches.length, 'trusted senders books, merging duplicates');
+              emails = await consolidateTrustedSenderBooks(client, bookId, matches.slice(1), emails);
+            }
+
+            set({ trustedSendersBookId: bookId, trustedSenderEmails: emails, trustedSendersLoaded: true, trustedSendersLoading: false });
+          } catch (error) {
+            debug.error('Failed to load trusted senders address book:', error);
+            set({ trustedSendersLoaded: true, trustedSendersLoading: false });
+          } finally {
+            trustedSendersLoadPromise = null;
           }
-          const bookId = book.id;
-          debug.log('contacts', 'Trusted senders book id:', bookId);
-          const contacts = await client.getContacts(bookId);
-          debug.log('contacts', 'Loaded', contacts.length, 'trusted sender contacts');
-          const emails = contacts.flatMap(c =>
-            c.emails ? Object.values(c.emails).map(e => e.address.toLowerCase().trim()) : []
-          ).filter(Boolean);
-          set({ trustedSendersBookId: bookId, trustedSenderEmails: emails, trustedSendersLoaded: true, trustedSendersLoading: false });
-        } catch (error) {
-          debug.error('Failed to load trusted senders address book:', error);
-          set({ trustedSendersLoaded: true, trustedSendersLoading: false });
-        }
+        })();
+
+        return trustedSendersLoadPromise;
       },
 
       addToTrustedSendersBook: async (client, email) => {