From 770c1a15c659b1eec7120f5e4d7103a53ca62f24 Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Wed, 6 May 2026 12:54:52 +0545 Subject: [PATCH 1/4] fix(OUT-3686): paginate-and-filter QBO customer email lookup QBO's /query parser silently mishandles RFC-legal special characters in PrimaryEmailAddr filters (confirmed for '+', and both '=' and 'LIKE' literal forms fail), returning 0 rows even when a matching customer exists. findOrCreateCustomer therefore missed existing QBO customers with plus-aliased emails and created duplicates. Replaces the WHERE-clause email filter with a paginated walk + JS-side match. The email never appears in the query, so any RFC-legal address works. Adds a sanitizedCompanyName parameter to disambiguate customers sharing an email across companies (one Copilot client can be enrolled in multiple companies); this absorbs the post-filter that previously did the same check in customer.service.ts. Outer wrapWithRetry intentionally dropped on getCustomerByEmail so a mid-walk 429 retries only the failing page, not the entire realm walk. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../quickbooks/customer/customer.service.ts | 20 +- src/utils/intuitAPI.ts | 46 +- test/unit/utils/intuitAPI.test.ts | 462 ++++++++++++++++++ 3 files changed, 509 insertions(+), 19 deletions(-) create mode 100644 test/unit/utils/intuitAPI.test.ts diff --git a/src/app/api/quickbooks/customer/customer.service.ts b/src/app/api/quickbooks/customer/customer.service.ts index bc0013cb..4ab6d92f 100644 --- a/src/app/api/quickbooks/customer/customer.service.ts +++ b/src/app/api/quickbooks/customer/customer.service.ts @@ -346,26 +346,22 @@ export class CustomerService extends BaseService { invoiceResource: InvoiceCreatedResponseType['data'] }) { const displayName = recipientInfo.displayName + const sanitizedCompanyName = recipientInfo.companyName + ? replaceSpecialCharsForQB(recipientInfo.companyName) + : undefined + // 2.1. search client in qb using recipient's email or display name let customer = recipientInfo.email - ? await intuitApiService.getCustomerByEmail(recipientInfo.email) + ? await intuitApiService.getCustomerByEmail( + recipientInfo.email, + sanitizedCompanyName, + ) : await intuitApiService.getACustomer( replaceSpecialCharsForQB(recipientInfo.displayName), undefined, true, ) - // 2.2. verify the matched customer has the same company name. This is needed because a single customer with same email can be part of multiple companies - const sanitizedCompanyName = recipientInfo.companyName - ? replaceSpecialCharsForQB(recipientInfo.companyName) - : undefined - if ( - customer && - (customer.CompanyName || undefined) !== sanitizedCompanyName - ) { - customer = undefined - } - addSyncBreadcrumb('Customer search in QBO', { found: !!customer, }) diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts index 732a54d4..ec1f86b1 100644 --- a/src/utils/intuitAPI.ts +++ b/src/utils/intuitAPI.ts @@ -296,20 +296,49 @@ export default class IntuitAPI { return CustomerQueryResponseSchema.parse(qbCustomers.Customer[0]) } + // QBO's query parser silently mishandles certain special characters (confirmed + // for '+', and '=' / 'LIKE' literal both fail) when filtering on + // PrimaryEmailAddr, returning 0 results even when a matching customer exists. + // To stay correct for any RFC-legal email, we never put the email in the WHERE + // clause: page through customers and match client-side instead. + // + // sanitizedCompanyName disambiguates customers sharing the same email across + // companies (one Copilot client can be enrolled in multiple companies). The + // CompanyName comparison uses the same `(value || undefined)` normalisation + // as the post-filter in customer.service.ts so the two layers cannot disagree. async _getCustomerByEmail( email: string, + sanitizedCompanyName: string | undefined, ): Promise { + const needle = email.trim().toLowerCase() + if (!needle) return + CustomLogger.info({ - obj: { email }, + obj: { email, sanitizedCompanyName }, message: `IntuitAPI#getCustomerByEmail | Customer query start for realmId: ${this.tokens.intuitRealmId}. Email: ${email}`, }) - const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE PrimaryEmailAddr = '${escapeForQBQuery(email)}' AND Active in (true, false)` - const qbCustomers = await this.customQuery(customerQuery) - if (!qbCustomers) return + const pageSize = 1000 + let startPosition = 1 - if (!qbCustomers.Customer) return - return CustomerQueryResponseSchema.parse(qbCustomers.Customer[0]) + while (true) { + const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE Active IN (true, false) STARTPOSITION ${startPosition} MAXRESULTS ${pageSize}` + const qbCustomers = await this.customQuery(customerQuery) + const customers = qbCustomers?.Customer ?? [] + if (customers.length === 0) return + + const match = customers.find((c: CustomerQueryResponseType) => { + const addr = c.PrimaryEmailAddr?.Address + if (typeof addr !== 'string') return false + if (addr.trim().toLowerCase() !== needle) return false + if ((c.CompanyName || undefined) !== sanitizedCompanyName) return false + return true + }) + if (match) return CustomerQueryResponseSchema.parse(match) + + if (customers.length < pageSize) return + startPosition += pageSize + } } /** @@ -892,7 +921,10 @@ export default class IntuitAPI { includeInactive?: boolean, ): Promise } = this.wrapWithRetry(this._getACustomer) as any - getCustomerByEmail = this.wrapWithRetry(this._getCustomerByEmail) + // Intentionally NOT wrapped in wrapWithRetry — a transient 429 mid-walk would + // replay from page 1 and amplify rate-limit pressure. The inner customQuery + // calls already retry on 429 (same reasoning as resolveUniqueCustomerName). + getCustomerByEmail = this._getCustomerByEmail.bind(this) getAnItem: { ( name: string, diff --git a/test/unit/utils/intuitAPI.test.ts b/test/unit/utils/intuitAPI.test.ts new file mode 100644 index 00000000..f501f200 --- /dev/null +++ b/test/unit/utils/intuitAPI.test.ts @@ -0,0 +1,462 @@ +/** + * Unit tests for `IntuitAPI._getCustomerByEmail` — the QBO customer lookup + * that paginates through all customers and matches by email client-side. + * + * Why this exists at all: QBO's `/query` parser silently mishandles certain + * special chars (confirmed for '+', and both `=` and `LIKE` literals fail) on + * `WHERE PrimaryEmailAddr = '...'`, returning 0 results even when a matching + * customer exists. To stay correct for any RFC-legal email, we never put the + * email in the WHERE clause — we walk the realm and filter in JS. + * + * Coverage focus: + * - Pagination terminates correctly on empty pages, partial pages, full + * pages followed by empty (off-by-one guard), and on first match. + * - Match is case-insensitive and whitespace-tolerant on both sides + * (search input AND stored value). + * - Malformed `PrimaryEmailAddr` rows do not throw — guards the + * defensive `typeof addr === 'string'` predicate that fixed the + * type-laundering bug found in review. + * - Empty/whitespace email short-circuits without calling QBO. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@sentry/nextjs', () => ({ + withScope: vi.fn(), + captureMessage: vi.fn(), + captureException: vi.fn(), +})) + +vi.mock('@/utils/logger', () => ({ + default: { + info: vi.fn(), + error: vi.fn(), + }, +})) + +// `_getCustomerByEmail` only uses `customQuery` from the instance; stubbing +// fetch helpers is defensive so any accidental fall-through fails loudly +// instead of hitting the network. +vi.mock('@/helper/fetch.helper', () => ({ + getFetcher: vi.fn(), + postFetcher: vi.fn(), +})) + +import IntuitAPI, { IntuitAPITokensType } from '@/utils/intuitAPI' + +const baseTokens: IntuitAPITokensType = { + accessToken: 'access', + refreshToken: 'refresh', + intuitRealmId: 'realm-1', + incomeAccountRef: 'income', + expenseAccountRef: 'expense', + assetAccountRef: 'asset', + serviceItemRef: 'service', + clientFeeRef: 'client-fee', +} + +// Builds a customer row in the shape QBO returns inside `QueryResponse.Customer`. +// `email: null` produces a row with no `PrimaryEmailAddr` at all (covers the +// "Address absent" branch). Any other value goes verbatim to test malformed +// shapes (string instead of object, etc.) without TS friction. +function row( + id: string, + email: string | null | { Address?: unknown }, + overrides: Record = {}, +) { + const base = { + Id: id, + SyncToken: '0', + Active: true, + CompanyName: undefined, + ...overrides, + } + if (email === null) return base + if (typeof email === 'string') { + return { ...base, PrimaryEmailAddr: { Address: email } } + } + return { ...base, PrimaryEmailAddr: email } +} + +// `customQuery` is a public field on IntuitAPI (`this.wrapWithRetry(this._customQuery)`). +// We replace it on the instance after construction so the test controls what +// each page returns. `getCustomerByEmail` is bound at construction time to the +// instance, so mutations to `instance.customQuery` are visible to it. +function makeApi(pages: Array) { + const api = new IntuitAPI(baseTokens) + const customQuery = vi.fn() + for (const page of pages) { + customQuery.mockResolvedValueOnce(page) + } + // Any call beyond the configured pages should make the test fail loudly + // instead of silently looping or returning undefined. + customQuery.mockImplementation(() => { + throw new Error('customQuery called more times than test configured') + }) + ;(api as unknown as { customQuery: unknown }).customQuery = customQuery + return { api, customQuery } +} + +describe('IntuitAPI#getCustomerByEmail', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns undefined without calling QBO when email is empty', async () => { + // Guard against a degenerate input wasting a full-realm walk. The early + // return short-circuits before any QBO request. + const { api, customQuery } = makeApi([]) + + const result = await api.getCustomerByEmail('', undefined) + + expect(result).toBeUndefined() + expect(customQuery).not.toHaveBeenCalled() + }) + + it('returns undefined without calling QBO when email is whitespace only', async () => { + // Same guard as above; trim happens before the empty check. + const { api, customQuery } = makeApi([]) + + const result = await api.getCustomerByEmail(' \t\n ', undefined) + + expect(result).toBeUndefined() + expect(customQuery).not.toHaveBeenCalled() + }) + + it('returns undefined when QBO returns an empty Customer array', async () => { + // `Customer` array present but empty → realm has no customers → terminate. + const { api, customQuery } = makeApi([{ Customer: [] }]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result).toBeUndefined() + expect(customQuery).toHaveBeenCalledTimes(1) + }) + + it('returns undefined when QBO returns no Customer field at all', async () => { + // QBO omits `Customer` entirely when the result is empty for some queries. + // The `?? []` coalesce treats this identically to an empty array. + const { api, customQuery } = makeApi([{}]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result).toBeUndefined() + expect(customQuery).toHaveBeenCalledTimes(1) + }) + + it('finds an exact-match customer on the first page', async () => { + // Happy path: realm returns a partial page (< pageSize), we find the match. + const { api, customQuery } = makeApi([ + { + Customer: [ + row('1', 'bob@example.com'), + row('2', 'alice@example.com'), + row('3', 'carol@example.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('2') + // Partial-page short-circuit: only one query, no second-page probe. + expect(customQuery).toHaveBeenCalledTimes(1) + }) + + it('matches case-insensitively (search uppercase, stored lowercase)', async () => { + // QBO is documented as case-insensitive on email — our client-side compare + // must respect that contract or we regress on emails like 'User@X.com'. + const { api } = makeApi([{ Customer: [row('1', 'alice@example.com')] }]) + + const result = await api.getCustomerByEmail('ALICE@EXAMPLE.COM', undefined) + + expect(result?.Id).toBe('1') + }) + + it('matches case-insensitively (stored uppercase, search lowercase)', async () => { + // Symmetric of the above; both sides are normalised before compare. + const { api } = makeApi([{ Customer: [row('1', 'Alice+Tag@Example.COM')] }]) + + const result = await api.getCustomerByEmail( + 'alice+tag@example.com', + undefined, + ) + + expect(result?.Id).toBe('1') + }) + + it('trims whitespace on both the input email and the stored Address', async () => { + // Both sides are trimmed before compare, so accidental whitespace from + // either Copilot payload or QBO storage does not cause a false negative. + const { api } = makeApi([{ Customer: [row('1', ' alice@example.com ')] }]) + + const result = await api.getCustomerByEmail( + '\nalice@example.com\t', + undefined, + ) + + expect(result?.Id).toBe('1') + }) + + it('finds the match on page 2 (advances STARTPOSITION correctly)', async () => { + // Pagination correctness: first 1000-row page has no match, second page + // contains the target. Verifies the loop advances by pageSize and does + // not re-scan page 1. + const page1 = { + Customer: Array.from({ length: 1000 }, (_, i) => + row(`p1-${i}`, `noise${i}@example.com`), + ), + } + const page2 = { + Customer: [row('target', 'alice+tag@example.com')], + } + const { api, customQuery } = makeApi([page1, page2]) + + const result = await api.getCustomerByEmail( + 'alice+tag@example.com', + undefined, + ) + + expect(result?.Id).toBe('target') + expect(customQuery).toHaveBeenCalledTimes(2) + // Sanity: the queries differ by STARTPOSITION (1 then 1001). + const firstQuery = customQuery.mock.calls[0][0] as string + const secondQuery = customQuery.mock.calls[1][0] as string + expect(firstQuery).toContain('STARTPOSITION 1 ') + expect(secondQuery).toContain('STARTPOSITION 1001 ') + }) + + it('terminates after a full page followed by an empty page (off-by-one guard)', async () => { + // Edge case the reviewer specifically asked to verify: if the LAST real + // page happens to contain exactly pageSize rows, the next iteration must + // see an empty page and terminate. Without this, the loop runs forever + // (or until QBO errors out). + const fullPage = { + Customer: Array.from({ length: 1000 }, (_, i) => + row(`p1-${i}`, `noise${i}@example.com`), + ), + } + const emptyPage = { Customer: [] } + const { api, customQuery } = makeApi([fullPage, emptyPage]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result).toBeUndefined() + expect(customQuery).toHaveBeenCalledTimes(2) + }) + + it('terminates on a partial page even when no match is found', async () => { + // < pageSize rows means QBO has no more pages. We must NOT issue a + // speculative next-page query that would just come back empty. + const { api, customQuery } = makeApi([ + { + Customer: [row('1', 'bob@example.com'), row('2', 'carol@example.com')], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result).toBeUndefined() + expect(customQuery).toHaveBeenCalledTimes(1) + }) + + it('skips rows with missing PrimaryEmailAddr without throwing', async () => { + // Defensive behaviour added after review: a row with no email field + // must not crash the predicate. Before the fix, accessing `.Address` + // on an unexpected shape would throw mid-find(). + const { api } = makeApi([ + { + Customer: [ + row('1', null), + row('2', null), + row('3', 'alice@example.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('3') + }) + + it('skips rows where PrimaryEmailAddr.Address is non-string without throwing', async () => { + // Same guard from a different angle: `Address` exists but is not a + // string (number, null, nested object). The `typeof addr === 'string'` + // check must short-circuit before `.trim()`. + const { api } = makeApi([ + { + Customer: [ + row('1', { Address: null }), + row('2', { Address: 12345 }), + row('3', { Address: undefined }), + row('4', 'alice@example.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('4') + }) + + it('returns the first match when multiple customers share the same email and pass the company predicate', async () => { + // QBO does not enforce email uniqueness across customers. With + // `sanitizedCompanyName=undefined` and rows that both have no + // CompanyName, both pass the company check, so `find` returns the + // first one in document order. Documenting this guards against a + // regression that re-orders results or returns all matches. + const { api } = makeApi([ + { + Customer: [ + row('first', 'alice@example.com'), + row('second', 'alice@example.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('first') + }) + + it('finds a customer whose email contains a "+" alias (the original bug)', async () => { + // The motivating scenario for this whole rewrite. With the old + // `WHERE PrimaryEmailAddr = '...'` query, QBO silently returned 0 rows + // for plus-aliased addresses. The walk-and-filter approach never puts + // the email in the query, so '+' is irrelevant. + const { api } = makeApi([ + { + Customer: [ + row('1', 'bob@example.com'), + row('2', 'json+smith@gmail.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail( + 'json+smith@gmail.com', + undefined, + ) + + expect(result?.Id).toBe('2') + }) + + it('emits a query that includes inactive customers', async () => { + // Drift recovery must consider deactivated customers — a customer might + // have been soft-deleted in QBO and the email lookup is the path that + // surfaces them so callers can decide whether to reactivate or duplicate. + const { api, customQuery } = makeApi([ + { Customer: [row('1', 'alice@example.com', { Active: false })] }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('1') + const query = customQuery.mock.calls[0][0] as string + expect(query).toMatch(/Active IN \(true, false\)/) + }) + + // Company-aware matching: a Copilot client can be enrolled in multiple + // companies, so the same email may exist on multiple QBO customers — only + // the one whose CompanyName matches the recipient's company is the right + // one to update. Without this filter the walker would return the first + // email-match and the post-filter in customer.service.ts would null it + // out, producing a duplicate-create regression. + + it('returns the customer whose CompanyName matches the sanitised company name', async () => { + // Two customers share the same email; only the one with CompanyName + // "Acme" should be returned when we search for that company. + const { api } = makeApi([ + { + Customer: [ + row('beta', 'alice@example.com', { CompanyName: 'Beta Inc' }), + row('acme', 'alice@example.com', { CompanyName: 'Acme' }), + row('gamma', 'alice@example.com', { CompanyName: 'Gamma' }), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', 'Acme') + + expect(result?.Id).toBe('acme') + }) + + it('skips email-matching customers with the wrong CompanyName and walks on to the next page', async () => { + // The motivating scenario: page 1 has an email-match with the wrong + // company, page 2 has the correct one. Walker must skip and continue, + // not stop at the first email match. + const page1 = { + Customer: [ + row('wrong', 'alice@example.com', { CompanyName: 'Beta Inc' }), + ...Array.from({ length: 999 }, (_, i) => + row(`p1-${i}`, `noise${i}@example.com`), + ), + ], + } + const page2 = { + Customer: [row('right', 'alice@example.com', { CompanyName: 'Acme' })], + } + const { api, customQuery } = makeApi([page1, page2]) + + const result = await api.getCustomerByEmail('alice@example.com', 'Acme') + + expect(result?.Id).toBe('right') + expect(customQuery).toHaveBeenCalledTimes(2) + }) + + it('rejects customers with a CompanyName when searching for an individual (no company)', async () => { + // Searching with `sanitizedCompanyName === undefined` means "individual + // customer, no company affiliation". A customer whose CompanyName is + // set must NOT match — picking them would mis-attribute an individual + // invoice to a company customer. Mirrors the post-filter semantics in + // customer.service.ts. + const { api } = makeApi([ + { + Customer: [ + row('company-attached', 'alice@example.com', { + CompanyName: 'Acme', + }), + row('individual', 'alice@example.com'), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('individual') + }) + + it('treats empty-string CompanyName as equivalent to undefined when searching for an individual', async () => { + // QBO sometimes returns CompanyName as "" instead of omitting the field. + // The `(c.CompanyName || undefined)` normalisation must coalesce empty + // string with undefined so the customer is still considered an + // individual. Without this, accidentally-blank QBO records would always + // miss and trigger a duplicate-create. + const { api } = makeApi([ + { + Customer: [row('1', 'alice@example.com', { CompanyName: '' })], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', undefined) + + expect(result?.Id).toBe('1') + }) + + it('returns undefined when no customer matches both the email and the company', async () => { + // Email exists in QBO but for a different company. The right behaviour + // is to return undefined so callers create a new customer for this + // (email, company) pair — not return the wrong-company match. + const { api } = makeApi([ + { + Customer: [ + row('beta', 'alice@example.com', { CompanyName: 'Beta Inc' }), + ], + }, + ]) + + const result = await api.getCustomerByEmail('alice@example.com', 'Acme') + + expect(result).toBeUndefined() + }) +}) From 1119d4becc606e84b30b625bfeaf9e0620d6dbe4 Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Wed, 6 May 2026 15:16:28 +0545 Subject: [PATCH 2/4] fix(OUT-3686): pin getCustomerByEmail walk to ORDERBY Id ASC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QBO's default ordering is MetaData.LastUpdatedTime DESC. Under that ordering, a customer updated between page fetches shifts to the front and can be pushed past our STARTPOSITION cursor — a false negative for a customer that genuinely exists, exactly the failure class this PR is fixing for plus-aliased emails. Flagged on review. Id is monotonic and immutable, so concurrent updates do not move rows and any customer created during the walk lands at the end of the cursor where we will still encounter it. Both race classes closed. Cost: newly-created customers land on the last page rather than page 1, so drift recovery for a fresh customer in a 10k realm walks all pages (~5s) instead of hitting on page 1 (~500ms). The perf cost is bounded and only fires on local-DB-miss paths — full stability over a faster but racier ordering. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/utils/intuitAPI.ts | 14 +++++++++++++- test/unit/utils/intuitAPI.test.ts | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts index ec1f86b1..3722108f 100644 --- a/src/utils/intuitAPI.ts +++ b/src/utils/intuitAPI.ts @@ -306,6 +306,18 @@ export default class IntuitAPI { // companies (one Copilot client can be enrolled in multiple companies). The // CompanyName comparison uses the same `(value || undefined)` normalisation // as the post-filter in customer.service.ts so the two layers cannot disagree. + // + // ORDERBY Id ASC pins the cursor to a stable, append-only key. QBO's default + // ordering is MetaData.LastUpdatedTime DESC — under that ordering, a customer + // updated mid-walk shifts to the front and can push an unscanned row past + // our STARTPOSITION cursor (false negative). Id is monotonic and immutable, + // so concurrent updates do not move rows and any customer created during the + // walk lands at the end of the cursor where we'll still encounter it. + // + // Tradeoff vs. CreateTime DESC: newly-created customers land on the LAST + // page rather than page 1, so drift recovery for a fresh customer in a 10k + // realm walks all pages (~5s) instead of hitting on page 1 (~500ms). The + // perf cost is bounded and acceptable; full stability is the priority. async _getCustomerByEmail( email: string, sanitizedCompanyName: string | undefined, @@ -322,7 +334,7 @@ export default class IntuitAPI { let startPosition = 1 while (true) { - const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE Active IN (true, false) STARTPOSITION ${startPosition} MAXRESULTS ${pageSize}` + const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE Active IN (true, false) ORDERBY Id ASC STARTPOSITION ${startPosition} MAXRESULTS ${pageSize}` const qbCustomers = await this.customQuery(customerQuery) const customers = qbCustomers?.Customer ?? [] if (customers.length === 0) return diff --git a/test/unit/utils/intuitAPI.test.ts b/test/unit/utils/intuitAPI.test.ts index f501f200..38d39f5f 100644 --- a/test/unit/utils/intuitAPI.test.ts +++ b/test/unit/utils/intuitAPI.test.ts @@ -356,6 +356,22 @@ describe('IntuitAPI#getCustomerByEmail', () => { expect(query).toMatch(/Active IN \(true, false\)/) }) + it('emits ORDERBY Id ASC so the cursor is stable across pages', async () => { + // Without an explicit order, QBO sorts by MetaData.LastUpdatedTime DESC. + // A customer updated between page fetches shifts under that ordering and + // can be skipped near a STARTPOSITION boundary. Id is monotonic and + // immutable, so concurrent updates do not move rows and concurrent + // creates always land at the end of the cursor. + const { api, customQuery } = makeApi([ + { Customer: [row('1', 'alice@example.com')] }, + ]) + + await api.getCustomerByEmail('alice@example.com', undefined) + + const query = customQuery.mock.calls[0][0] as string + expect(query).toMatch(/ORDERBY Id ASC/) + }) + // Company-aware matching: a Copilot client can be enrolled in multiple // companies, so the same email may exist on multiple QBO customers — only // the one whose CompanyName matches the recipient's company is the right From 6cf76e43f1dec150873df3217abc754dad261f41 Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Wed, 6 May 2026 16:54:42 +0545 Subject: [PATCH 3/4] chore(OUT-3686): update sanitizedCompanyName to be optional param --- src/utils/intuitAPI.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts index 3722108f..fd975550 100644 --- a/src/utils/intuitAPI.ts +++ b/src/utils/intuitAPI.ts @@ -320,7 +320,7 @@ export default class IntuitAPI { // perf cost is bounded and acceptable; full stability is the priority. async _getCustomerByEmail( email: string, - sanitizedCompanyName: string | undefined, + sanitizedCompanyName?: string, ): Promise { const needle = email.trim().toLowerCase() if (!needle) return From 119d5c2a41f74a10c5afc10667c9ea0ba07c4e74 Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Thu, 7 May 2026 12:35:24 +0545 Subject: [PATCH 4/4] chore(OUT-3686): condense getCustomerByEmail rationale comment Co-Authored-By: Claude Opus 4.7 (1M context) --- src/utils/intuitAPI.ts | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts index fd975550..e652b33a 100644 --- a/src/utils/intuitAPI.ts +++ b/src/utils/intuitAPI.ts @@ -296,28 +296,11 @@ export default class IntuitAPI { return CustomerQueryResponseSchema.parse(qbCustomers.Customer[0]) } - // QBO's query parser silently mishandles certain special characters (confirmed - // for '+', and '=' / 'LIKE' literal both fail) when filtering on - // PrimaryEmailAddr, returning 0 results even when a matching customer exists. - // To stay correct for any RFC-legal email, we never put the email in the WHERE - // clause: page through customers and match client-side instead. - // - // sanitizedCompanyName disambiguates customers sharing the same email across - // companies (one Copilot client can be enrolled in multiple companies). The - // CompanyName comparison uses the same `(value || undefined)` normalisation - // as the post-filter in customer.service.ts so the two layers cannot disagree. - // - // ORDERBY Id ASC pins the cursor to a stable, append-only key. QBO's default - // ordering is MetaData.LastUpdatedTime DESC — under that ordering, a customer - // updated mid-walk shifts to the front and can push an unscanned row past - // our STARTPOSITION cursor (false negative). Id is monotonic and immutable, - // so concurrent updates do not move rows and any customer created during the - // walk lands at the end of the cursor where we'll still encounter it. - // - // Tradeoff vs. CreateTime DESC: newly-created customers land on the LAST - // page rather than page 1, so drift recovery for a fresh customer in a 10k - // realm walks all pages (~5s) instead of hitting on page 1 (~500ms). The - // perf cost is bounded and acceptable; full stability is the priority. + // QBO's parser mishandles special chars on PrimaryEmailAddr filters, so we + // page and match client-side. sanitizedCompanyName disambiguates the same + // email across companies; normalisation matches customer.service.ts. + // ORDERBY Id ASC pins a stable cursor — QBO's default (LastUpdatedTime DESC) + // lets a mid-walk update shift a row past STARTPOSITION (false negative). async _getCustomerByEmail( email: string, sanitizedCompanyName?: string,