Skip to content

Commit 5407a0c

Browse files
waleedlatif1claude
andcommitted
fix(quickbooks): stop item full updates from corrupting inventory and transaction history
Two of these silently rewrite a customer's accounting records. An Item full update echoes the record read back from QuickBooks. Intuit documents InvStartDate asymmetrically: "For read operations, the date returned in this field is always the originally provided inventory start date. For update operations, the date supplied is interpreted as the inventory adjust date, is stored as such in the underlying data model, and is reflected in the QuickBooks Online UI." QtyOnHand is re-asserted the same way. Both are "Required for Inventory type items", so neither can simply be dropped from the body — refuse the update instead, matching create_item's existing Service/NonInventory restriction. Intuit also documents inactivation as "Not valid for Category item types", so an Active change on a Category is refused too. The Item update also posted to a bare endpoint. Intuit: "Add the query parameter, include=donotupdateaccountontxns, to the endpoint to supress updating the income or expense account on any existing transactions associated with this Item object." Without it, changing an item's account rewrote every historical transaction linked to it. The parameter is documented on the Item update alone, so it is not applied to any other entity. Also aligns the shared plumbing with the documented model: - PhysicalAddress documents Line1-Line5; the write map carried only Line1/Line2, so an address Sim had just read could not be written back. - Fault.type (ValidationFault / SystemFault / AuthenticationFault / AuthorizationFault) was discarded, hiding the classification that separates a bad payload from a dead token. Matched by prefix because Intuit's pages disagree between "ValidationFault" and type="Validation". - The query string used the form-encoded "+" for spaces; Intuit's own example percent-encodes them. - MAXRESULTS was capped at 100 against a documented maximum of 1,000. - Validates documented constraints locally: DisplayName <=500, Item.Name <=100 with no tabs, new lines, or colons, and an email address Intuit can store. - update_item's activeStatus and update_employee's displayName now carry the Category and Payroll caveats Intuit documents. Adds the missing query-builder coverage and a full-update test that would catch feeding the sanitized record into the merge, which would null every vendor's TaxIdentifier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RB5M2gk7zZgGomnQqr7WRB
1 parent 07f9190 commit 5407a0c

13 files changed

Lines changed: 445 additions & 16 deletions

‎apps/sim/tools/quickbooks/api_accuracy.test.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@ import { quickbooksReadMasterDataTool } from '@/tools/quickbooks/read_master_dat
2121
import { quickbooksReadPurchasingTransactionsTool } from '@/tools/quickbooks/read_purchasing_transactions'
2222
import { quickbooksReadSalesTransactionsTool } from '@/tools/quickbooks/read_sales_transactions'
2323
import type { QuickBooksAttachmentTargetType } from '@/tools/quickbooks/types'
24+
import {
25+
buildQuickBooksMasterDataQueryUrl,
26+
buildQuickBooksQueryUrl,
27+
buildQuickBooksSalesQueryUrl,
28+
} from '@/tools/quickbooks/utils'
29+
import {
30+
parseQuickBooksAddress,
31+
quickBooksDisplayName,
32+
quickBooksEmailAddress,
33+
quickBooksItemName,
34+
} from '@/tools/quickbooks/values'
2435

2536
describe('QuickBooks credential authority', () => {
2637
it('declares both credential-derived routing parameters on every tool', () => {
@@ -332,3 +343,115 @@ describe('QuickBooks locale-dependent item accounts', () => {
332343
).toEqual({ Name: 'Conseil', Type: 'Service' })
333344
})
334345
})
346+
347+
describe('QuickBooks documented query language', () => {
348+
const auth = {
349+
accessToken: 'token',
350+
realmId: '123',
351+
quickBooksEnvironment: 'sandbox',
352+
} as const
353+
354+
function getQueryStatement(url: URL): string {
355+
return url.searchParams.get('query') ?? ''
356+
}
357+
358+
it('percent-encodes spaces rather than using the form-encoded plus', () => {
359+
const url = buildQuickBooksQueryUrl(auth, 'Customer', 1, 25)
360+
361+
expect(url.search).not.toContain('+')
362+
expect(url.search).toContain(
363+
'query=SELECT%20*%20FROM%20Customer%20STARTPOSITION%201%20MAXRESULTS%2025'
364+
)
365+
})
366+
367+
it('emits one-based STARTPOSITION and MAXRESULTS in the documented order', () => {
368+
expect(getQueryStatement(buildQuickBooksQueryUrl(auth, 'Invoice', 26, 25))).toBe(
369+
'SELECT * FROM Invoice STARTPOSITION 26 MAXRESULTS 25'
370+
)
371+
})
372+
373+
it('renders a boolean Active filter unquoted', () => {
374+
expect(
375+
getQueryStatement(
376+
buildQuickBooksMasterDataQueryUrl({
377+
...auth,
378+
recordType: 'vendor',
379+
readMode: 'list',
380+
activeStatus: 'inactive',
381+
})
382+
)
383+
).toBe('SELECT * FROM Vendor WHERE Active = false STARTPOSITION 1 MAXRESULTS 25')
384+
})
385+
386+
it('omits the WHERE clause when no filter is requested', () => {
387+
expect(
388+
getQueryStatement(
389+
buildQuickBooksMasterDataQueryUrl({ ...auth, recordType: 'item', readMode: 'list' })
390+
)
391+
).toBe('SELECT * FROM Item STARTPOSITION 1 MAXRESULTS 25')
392+
})
393+
394+
it('escapes backslashes before apostrophes in a filter value', () => {
395+
expect(
396+
getQueryStatement(
397+
buildQuickBooksSalesQueryUrl({
398+
...auth,
399+
transactionType: 'invoice',
400+
readMode: 'list',
401+
customerId: "a\\b'c",
402+
})
403+
)
404+
).toBe("SELECT * FROM Invoice WHERE CustomerRef = 'a\\\\b\\'c' STARTPOSITION 1 MAXRESULTS 25")
405+
})
406+
407+
it('accepts the documented maximum page size of 1000', () => {
408+
expect(getQueryStatement(buildQuickBooksQueryUrl(auth, 'Customer', 1, 1000))).toBe(
409+
'SELECT * FROM Customer STARTPOSITION 1 MAXRESULTS 1000'
410+
)
411+
expect(() => buildQuickBooksQueryUrl(auth, 'Customer', 1, 1001)).toThrow(
412+
'maxResults must be an integer from 1 through 1000'
413+
)
414+
})
415+
})
416+
417+
describe('QuickBooks documented field constraints', () => {
418+
it('writes back every address line QuickBooks returns', () => {
419+
expect(
420+
parseQuickBooksAddress(
421+
{ Line1: '1', Line2: '2', Line3: '3', Line4: '4', Line5: '5' },
422+
'billingAddress'
423+
)
424+
).toEqual({ Line1: '1', Line2: '2', Line3: '3', Line4: '4', Line5: '5' })
425+
expect(parseQuickBooksAddress({ line3: '3' }, 'billingAddress')).toEqual({ Line3: '3' })
426+
})
427+
428+
it('rejects an item name QuickBooks documents as invalid', () => {
429+
expect(() => quickBooksItemName('Parent:Child')).toThrow(
430+
'name cannot include tabs, new lines, or colons'
431+
)
432+
expect(() => quickBooksItemName('a'.repeat(101))).toThrow('name cannot exceed 100 characters')
433+
expect(quickBooksItemName('Consulting')).toBe('Consulting')
434+
})
435+
436+
it('rejects a display name past the documented 500-character maximum', () => {
437+
expect(() => quickBooksDisplayName('a'.repeat(501), 'displayName')).toThrow(
438+
'displayName cannot exceed 500 characters'
439+
)
440+
expect(quickBooksDisplayName('a'.repeat(500), 'displayName')).toHaveLength(500)
441+
})
442+
443+
it('rejects an email address QuickBooks cannot store', () => {
444+
expect(() => quickBooksEmailAddress('not-an-email')).toThrow(
445+
'Email address must be a valid address such as name@example.com'
446+
)
447+
expect(() => quickBooksEmailAddress('a@b@c.com')).toThrow(
448+
'Email address must be a valid address such as name@example.com'
449+
)
450+
expect(() => quickBooksEmailAddress(`${'a'.repeat(95)}@example.com`)).toThrow(
451+
'Email address cannot exceed 100 characters'
452+
)
453+
expect(quickBooksEmailAddress('billing@example.com')).toEqual({
454+
Address: 'billing@example.com',
455+
})
456+
})
457+
})

‎apps/sim/tools/quickbooks/create_customer.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import {
2020
optionalQuickBooksString,
2121
parseQuickBooksAddress,
22+
quickBooksDisplayName,
2223
quickBooksEmailAddress,
2324
quickBooksPhoneNumber,
2425
} from '@/tools/quickbooks/values'
@@ -129,7 +130,7 @@ export const quickbooksCreateCustomerTool: ToolConfig<
129130
method: 'POST',
130131
headers: (params) => getQuickBooksToolHeaders(params.accessToken, 'application/json'),
131132
body: (params) => {
132-
const displayName = optionalQuickBooksString(params.displayName)
133+
const displayName = quickBooksDisplayName(params.displayName, 'displayName')
133134
const givenName = optionalQuickBooksString(params.givenName)
134135
const familyName = optionalQuickBooksString(params.familyName)
135136
if (displayName === undefined && givenName === undefined && familyName === undefined) {

‎apps/sim/tools/quickbooks/create_employee.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import {
2020
optionalQuickBooksString,
2121
parseQuickBooksAddress,
22+
quickBooksDisplayName,
2223
quickBooksEmailAddress,
2324
quickBooksPhoneNumber,
2425
} from '@/tools/quickbooks/values'
@@ -129,7 +130,7 @@ export const quickbooksCreateEmployeeTool: ToolConfig<
129130
throw new Error('At least one of givenName or familyName must be supplied')
130131
}
131132
return filterUndefined({
132-
DisplayName: optionalQuickBooksString(params.displayName),
133+
DisplayName: quickBooksDisplayName(params.displayName, 'displayName'),
133134
GivenName: givenName,
134135
FamilyName: familyName,
135136
PrimaryEmailAddr: quickBooksEmailAddress(params.primaryEmail),

‎apps/sim/tools/quickbooks/create_item.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@/tools/quickbooks/utils'
1515
import {
1616
optionalQuickBooksString,
17+
quickBooksItemName,
1718
quickBooksReference,
1819
quickBooksWritableItemType,
1920
requiredQuickBooksString,
@@ -52,7 +53,7 @@ export const quickbooksCreateItemTool: ToolConfig<
5253
type: 'string',
5354
required: true,
5455
visibility: 'user-or-llm',
55-
description: 'Unique item name',
56+
description: 'Unique item name, up to 100 characters, without tabs, new lines, or colons',
5657
},
5758
itemType: {
5859
type: 'string',
@@ -128,7 +129,7 @@ export const quickbooksCreateItemTool: ToolConfig<
128129
const incomeAccountId = optionalQuickBooksString(params.incomeAccountId)
129130
const expenseAccountId = optionalQuickBooksString(params.expenseAccountId)
130131
return filterUndefined({
131-
Name: requiredQuickBooksString(params.name, 'name'),
132+
Name: quickBooksItemName(requiredQuickBooksString(params.name, 'name')),
132133
Type: type,
133134
IncomeAccountRef:
134135
incomeAccountId === undefined

‎apps/sim/tools/quickbooks/create_vendor.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import {
1717
optionalQuickBooksString,
1818
parseQuickBooksAddress,
19+
quickBooksDisplayName,
1920
quickBooksEmailAddress,
2021
quickBooksPhoneNumber,
2122
} from '@/tools/quickbooks/values'
@@ -132,7 +133,7 @@ export const quickbooksCreateVendorTool: ToolConfig<
132133
method: 'POST',
133134
headers: (params) => getQuickBooksToolHeaders(params.accessToken, 'application/json'),
134135
body: (params) => {
135-
const displayName = optionalQuickBooksString(params.displayName)
136+
const displayName = quickBooksDisplayName(params.displayName, 'displayName')
136137
const givenName = optionalQuickBooksString(params.givenName)
137138
const familyName = optionalQuickBooksString(params.familyName)
138139
if (displayName === undefined && givenName === undefined && familyName === undefined) {

‎apps/sim/tools/quickbooks/fault.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,39 @@ describe('QuickBooks fault handling', () => {
6868
expect(formatQuickBooksFaultDetail(elementOnly!)).toBe('element: SyncToken')
6969
})
7070

71+
it.each([
72+
['ValidationFault', 'ValidationFault'],
73+
['Validation', 'ValidationFault'],
74+
['AuthenticationFault', 'AuthenticationFault'],
75+
['Authentication', 'AuthenticationFault'],
76+
['AuthorizationFault', 'AuthorizationFault'],
77+
['SystemFault', 'SystemFault'],
78+
])('reports the documented fault classification for type %s', (type, expected) => {
79+
const fault = sanitizeQuickBooksFaultData({
80+
Fault: { type, Error: [{ code: '6240', Message: 'Duplicate Name Exists Error' }] },
81+
})
82+
83+
expect(fault?.Fault.type).toBe(expected)
84+
expect(formatQuickBooksFaultDetail(fault!)).toBe(
85+
`${expected}: 6240: Duplicate Name Exists Error`
86+
)
87+
})
88+
89+
it('preserves the classification across repeated sanitization', () => {
90+
const first = sanitizeQuickBooksFaultData({
91+
Fault: { type: 'AuthenticationFault', Error: [{ Message: 'message' }] },
92+
})
93+
94+
expect(sanitizeQuickBooksFaultData(first)?.Fault.type).toBe('AuthenticationFault')
95+
})
96+
97+
it('omits the classification when Intuit does not send one', () => {
98+
const fault = sanitizeQuickBooksFaultData({ Fault: { Error: [{ Message: 'message' }] } })
99+
100+
expect(fault?.Fault).not.toHaveProperty('type')
101+
expect(formatQuickBooksFaultDetail(fault!)).toBe('message')
102+
})
103+
71104
it.each([null, [], {}, { Fault: {} }, { Fault: { Error: [] } }])(
72105
'does not claim malformed fault payloads: %j',
73106
(payload) => {

‎apps/sim/tools/quickbooks/fault.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ const QUICKBOOKS_MAX_FAULT_FIELD_CHARS = 500
1212
/** Documented Intuit fault fields retained during sanitization. */
1313
const QUICKBOOKS_FAULT_FIELDS = ['code', 'Message', 'Detail', 'element'] as const
1414

15+
/**
16+
* Documented `Fault.type` classifications, which separate a rejected payload
17+
* from an expired or unauthorized token. Intuit's own pages render these both
18+
* with and without the `Fault` suffix — `type="ValidationFault"` in one sample,
19+
* `type="Validation"` in another — so they are matched by prefix and reported
20+
* under a single spelling.
21+
*/
22+
const QUICKBOOKS_FAULT_TYPES = ['Authentication', 'Authorization', 'Validation', 'System'] as const
23+
24+
function normalizeQuickBooksFaultType(value: unknown): string | undefined {
25+
const type = typeof value === 'string' ? value.trim() : ''
26+
if (!type) return undefined
27+
const known = QUICKBOOKS_FAULT_TYPES.find((candidate) => type.startsWith(candidate))
28+
return known ? `${known}Fault` : truncate(type, QUICKBOOKS_MAX_FAULT_FIELD_CHARS, '')
29+
}
30+
1531
/**
1632
* Intuit error code and message for an outdated `SyncToken`.
1733
* @see Fault sample `{"Message": "Stale Object Error", "code": "5010"}`
@@ -25,6 +41,8 @@ const QUICKBOOKS_STALE_OBJECT_GUIDANCE =
2541
export interface SanitizedQuickBooksFault {
2642
Fault: {
2743
Error: Array<Record<string, string>>
44+
/** Documented Intuit fault classification, normalized by prefix. */
45+
type?: string
2846
/**
2947
* Count of `Error` entries dropped by {@link QUICKBOOKS_MAX_FAULT_ERRORS}.
3048
* Preserved across repeated sanitization so a value that round-trips
@@ -72,9 +90,12 @@ export function sanitizeQuickBooksFaultData(data: unknown): SanitizedQuickBooksF
7290
const carriedOmitted = typeof priorOmitted === 'number' && priorOmitted > 0 ? priorOmitted : 0
7391
const omittedErrorCount = carriedOmitted + Math.max(0, usableErrorCount - sanitizedErrors.length)
7492

93+
const type = normalizeQuickBooksFaultType((fault as Record<string, unknown>).type)
94+
7595
return {
7696
Fault: {
7797
Error: sanitizedErrors,
98+
...(type ? { type } : {}),
7899
...(omittedErrorCount > 0 ? { omittedErrorCount } : {}),
79100
},
80101
}
@@ -91,7 +112,8 @@ function hasQuickBooksStaleObjectError(fault: SanitizedQuickBooksFault): boolean
91112
/**
92113
* Renders a sanitized fault as a single human-readable detail string.
93114
*
94-
* Each entry becomes `code: Message: Detail (element)`. Entries dropped by the
115+
* The documented fault classification leads, then each entry becomes
116+
* `code: Message: Detail (element)`. Entries dropped by the
95117
* sanitizer are reported as a trailing count, and a stale-`SyncToken` fault
96118
* gains explicit remediation guidance because retrying is never correct.
97119
*/
@@ -110,6 +132,7 @@ export function formatQuickBooksFaultDetail(fault: SanitizedQuickBooksFault): st
110132

111133
const omitted = fault.Fault.omittedErrorCount ?? 0
112134
return [
135+
details.length > 0 && fault.Fault.type ? `${fault.Fault.type}:` : '',
113136
details.join('; '),
114137
omitted > 0
115138
? `(${omitted} additional QuickBooks error${omitted === 1 ? '' : 's'} omitted)`

0 commit comments

Comments
 (0)