fix(OUT-3971): eliminate flaky synced_contacts duplicate-key race

src/features/invoice-sync/lib/SyncedContacts.service.ts

@@ -140,42 +140,75 @@ class SyncedContactsService extends AuthenticatedXeroService {
     }
 
     const syncLogsService = new SyncLogsService(this.user, this.connection)
+    const clientOrCompanyId = isNotPlaceholderCompany ? companyId : z.string().parse(client?.id)
+    const contactUserType = isNotPlaceholderCompany
+      ? SyncedContactUserType.COMPANY
+      : SyncedContactUserType.CLIENT
 
     try {
       const contact = await this.xero.createContact(this.connection.tenantId, contactPayload)
+      const contactId = z.string().parse(contact.contactID)
 
-      await this.db.insert(syncedContacts).values({
-        portalId: this.user.portalId,
-        clientOrCompanyId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id),
-        userType: isNotPlaceholderCompany
-          ? SyncedContactUserType.COMPANY
-          : SyncedContactUserType.CLIENT,
-        contactId: z.string().parse(contact.contactID),
-        tenantId: this.connection.tenantId,
-      })
+      // Ignore a duplicate from a concurrent sync instead of failing.
+      const [inserted] = await this.db
+        .insert(syncedContacts)
+        .values({
+          portalId: this.user.portalId,
+          clientOrCompanyId,
+          userType: contactUserType,
+          contactId,
+          tenantId: this.connection.tenantId,
+        })
+        .onConflictDoNothing({
+          target: [
+            syncedContacts.portalId,
+            syncedContacts.tenantId,
+            syncedContacts.clientOrCompanyId,
+          ],
+        })
+        .returning({ contactId: syncedContacts.contactId })
+
+      // A concurrent sync won the insert; reuse its stored contact.
+      if (!inserted) {
+        const [existing] = await this.db
+          .select({ contactId: syncedContacts.contactId })
+          .from(syncedContacts)
+          .where(
+            and(
+              eq(syncedContacts.portalId, this.user.portalId),
+              eq(syncedContacts.tenantId, this.connection.tenantId),
+              eq(syncedContacts.clientOrCompanyId, clientOrCompanyId),
+            ),
+          )
+        logger.info(
+          'SyncedContactsService#createContact :: Mapping already exists, reusing contact',
+          existing?.contactId,
+        )
+        return { ...contact, contactID: z.string().parse(existing?.contactId) }
+      }
 
       await syncLogsService.createSyncLog({
         entityType: SyncEntityType.CUSTOMER,
         eventType: SyncEventType.CREATED,
         status: SyncStatus.SUCCESS,
         syncDate: new Date(),
-        copilotId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id),
+        copilotId: clientOrCompanyId,
         xeroId: contact.contactID,
         customerName: contact.name,
         customerEmail: contact.emailAddress,
       })
 
       return {
         ...contact,
-        contactID: z.string().parse(contact.contactID),
+        contactID: contactId,
       }
     } catch (error: unknown) {
       throw new APIError('Failed to create synced contact', status.INTERNAL_SERVER_ERROR, {
         error,
         failedSyncLogPayload: {
           entityType: SyncEntityType.CUSTOMER,
           eventType: SyncEventType.CREATED,
-          copilotId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id),
+          copilotId: clientOrCompanyId,
           customerName: contactPayload.name,
           customerEmail: contactPayload.emailAddress,
         },

src/features/invoice-sync/lib/SyncedInvoices.service.ts

@@ -38,6 +38,9 @@ class SyncedInvoicesService extends AuthenticatedXeroService {
     xeroInvoiceId: string | null
     salesAccountId: string | null
     status: NonNullable<SyncedInvoiceCreatePayload['status']>
+    // Exposed so callers reuse it instead of resolving the contact again.
+    customerName?: string
+    customerEmail?: string
   }> {
     logger.info('SyncedInvoicesService#syncInvoiceToXero :: Syncing invoice to xero:', data.id)
 
@@ -160,7 +163,7 @@ class SyncedInvoicesService extends AuthenticatedXeroService {
     logger.info(
       `SyncedInvoicesService#syncInvoiceToXero :: Synced Copilot invoice ${syncedInvoiceRecord.copilotInvoiceId} (${syncedInvoice?.invoiceNumber}) to Xero invoice ${syncedInvoiceRecord.xeroInvoiceId} for portalId ${this.connection.portalId}`,
     )
-    return syncedInvoiceRecord
+    return { ...syncedInvoiceRecord, customerName, customerEmail }
   }
 
   private async createMissingXeroInvoice(copilotInvoiceId: string) {
@@ -169,17 +172,23 @@ class SyncedInvoicesService extends AuthenticatedXeroService {
       copilotInvoiceId,
     )
     const invoice = await this.copilot.getInvoice(copilotInvoiceId)
-    const [record, contact] = await Promise.all([
-      this.syncInvoiceToXero(invoice),
-      this.getContact(invoice),
-    ])
-    if (!('xeroInvoiceId' in record) || !record.xeroInvoiceId) {
+    // Reuse the contact syncInvoiceToXero resolved; a second fetch here races it.
+    const record = await this.syncInvoiceToXero(invoice)
+    if (!record.xeroInvoiceId) {
       throw new APIError(
         `Failed to create Xero invoice for Copilot invoice ${copilotInvoiceId}`,
         status.INTERNAL_SERVER_ERROR,
       )
     }
 
+    // Already-synced short-circuit returns no contact; fetch it for the log.
+    let { customerName, customerEmail } = record
+    if (!customerName && !customerEmail) {
+      const contact = await this.getContact(invoice)
+      customerName = contact.name
+      customerEmail = contact.emailAddress
+    }
+
     const syncLogsService = new SyncLogsService(this.user, this.connection)
     await syncLogsService.createSyncLog({
       entityType: SyncEntityType.INVOICE,
@@ -191,8 +200,8 @@ class SyncedInvoicesService extends AuthenticatedXeroService {
       invoiceNumber: invoice.number,
       copilotId: invoice.id,
       xeroId: record.xeroInvoiceId,
-      customerEmail: contact.emailAddress,
-      customerName: contact.name,
+      customerEmail,
+      customerName,
     })
 
     return record

test/integration/syncedContacts/createContactIdempotency.test.ts

@@ -0,0 +1,67 @@
+import AuthService from '@auth/lib/Auth.service'
+import SyncedContactsService from '@invoice-sync/lib/SyncedContacts.service'
+import { TEST_CLIENT, TEST_COMPANY, TEST_TOKENS, TEST_XERO_CONTACT } from '@test/helpers/constants'
+import { createMockXeroAPI } from '@test/helpers/mocks'
+import { seedConnectedPortal, seedSyncedContact } from '@test/helpers/seed'
+import { setupWebhookTest } from '@test/helpers/webhookTestSetup'
+import { eq } from 'drizzle-orm'
+import { describe, expect, it, vi } from 'vitest'
+import db from '@/db'
+import { syncedContacts } from '@/db/schema/syncedContacts.schema'
+import { SyncEntityType, syncLogs } from '@/db/schema/syncLogs.schema'
+import User from '@/lib/copilot/models/User.model'
+
+// A concurrent sync already stored this mapping; our insert collides on it.
+const WINNER_CONTACT_ID = TEST_XERO_CONTACT.id
+// The losing sync still creates its own (orphan) Xero contact before inserting.
+const ORPHAN_CONTACT_ID = '99999999-9999-4999-8999-999999999999'
+
+const CLIENT = {
+  id: TEST_CLIENT.id,
+  givenName: TEST_CLIENT.givenName,
+  familyName: TEST_CLIENT.familyName,
+  email: TEST_CLIENT.email,
+  companyIds: [TEST_COMPANY.id],
+  status: 'active',
+  avatarImageUrl: null,
+  fallbackColor: null,
+  createdAt: '2026-01-01T00:00:00.000Z',
+}
+
+describe('SyncedContactsService.createContact — concurrent-safe get-or-create', () => {
+  setupWebhookTest(() => ({
+    xero: createMockXeroAPI({
+      createContact: vi.fn().mockResolvedValue({
+        contactID: ORPHAN_CONTACT_ID,
+        name: `${TEST_CLIENT.givenName} ${TEST_CLIENT.familyName}`,
+        emailAddress: TEST_CLIENT.email,
+      }),
+    }),
+  }))
+
+  it('returns the existing mapping instead of failing on a duplicate insert', async () => {
+    await seedConnectedPortal()
+    await seedSyncedContact({ contactId: WINNER_CONTACT_ID })
+
+    const user = await User.authenticate(TEST_TOKENS.webhook)
+    const connection = await new AuthService(user).authorizeXeroForCopilotWorkspace()
+    const service = new SyncedContactsService(user, connection)
+
+    const contact = await service.createContact(CLIENT)
+
+    // Returns the stored winner, not the orphan just created in Xero.
+    expect(contact.contactID).toBe(WINNER_CONTACT_ID)
+
+    // The unique mapping is untouched — no duplicate row.
+    const rows = await db.select().from(syncedContacts)
+    expect(rows).toHaveLength(1)
+    expect(rows[0].contactId).toBe(WINNER_CONTACT_ID)
+
+    // A conflict is a no-op, not a create — no customer sync log is written.
+    const customerLogs = await db
+      .select()
+      .from(syncLogs)
+      .where(eq(syncLogs.entityType, SyncEntityType.CUSTOMER))
+    expect(customerLogs).toHaveLength(0)
+  })
+})

test/integration/webhook/invoicePaid/missingXeroInvoice.test.ts

@@ -34,6 +34,9 @@ describe('POST /api/webhook — invoice.paid missing Xero invoice', () => {
     expect(apis.xero.createInvoice).toHaveBeenCalledTimes(1)
     expect(apis.xero.markInvoicePaid).toHaveBeenCalledTimes(1)
 
+    // Contact resolved once; the old path resolved it twice and self-raced.
+    expect(apis.xero.createContact).toHaveBeenCalledTimes(1)
+
     // Invoice row now mapped to Xero and marked success.
     const invoices = await db.select().from(syncedInvoices)
     expect(invoices).toHaveLength(1)