From e79feae8e57c2eee513ec87887e2147b5d780a45 Mon Sep 17 00:00:00 2001 From: Todd Hebebrand Date: Sun, 12 Jul 2026 01:03:14 -0600 Subject: [PATCH] fix(ai-tools): structured VALIDATION_ERROR for manage_* billing-domain tools (#2362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit manage_quotes (and its siblings manage_invoices, manage_contracts, manage_catalog) let malformed calls escape as raw HTTP 500s: the flat tool input schema marks every id/payload optional, so a missing quoteId was coerced via String(undefined) into the literal "undefined" and died downstream as an opaque uuid/DB error, and payload casts like `input.line as QuoteLineInput` let ZodError-free garbage straight into the services. - New shared apps/api/src/services/aiToolValidation.ts: per-action required-param presence checks (run BEFORE any coercion) and ZodError -> structured `{ error, code: "VALIDATION_ERROR" }` mapping with self-describing paths ("line.sourceType: ..."). - manage_quotes now parses input/patch/block/line payloads with the same shared Zod schemas the HTTP quote routes use (one source of truth), and add_catalog_line validates catalogItemId presence + guid before coercion — a partNumber-only call gets a clean error instead of a 500. - Tool descriptions now enumerate required fields per action; the line/input/block params document required + optional fields (sourceType and taxable were previously undiscoverable), and partNumber is explicitly documented as a stored override, NOT a lookup key. - Same missing-param guard + ZodError mapping swept across manage_invoices, manage_contracts, manage_catalog. - Unit tests per aiTools conventions: missing quoteId, missing line.sourceType/taxable, partNumber-only add_catalog_line, non-uuid catalogItemId, mismatched block content, plus sweep coverage for the sibling tools — each asserts the structured error, not a throw. Closes #2362 Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/services/aiToolValidation.ts | 56 ++++++ .../aiToolsBilling.manageInvoices.test.ts | 19 ++ apps/api/src/services/aiToolsBilling.ts | 39 +++- .../aiToolsCatalog.manageCatalog.test.ts | 9 + apps/api/src/services/aiToolsCatalog.ts | 30 +++- .../aiToolsContracts.manageContracts.test.ts | 9 + apps/api/src/services/aiToolsContracts.ts | 33 +++- apps/api/src/services/aiToolsQuotes.test.ts | 139 ++++++++++++++- apps/api/src/services/aiToolsQuotes.ts | 168 ++++++++++++++---- 9 files changed, 455 insertions(+), 47 deletions(-) create mode 100644 apps/api/src/services/aiToolValidation.ts diff --git a/apps/api/src/services/aiToolValidation.ts b/apps/api/src/services/aiToolValidation.ts new file mode 100644 index 0000000000..9831d31180 --- /dev/null +++ b/apps/api/src/services/aiToolValidation.ts @@ -0,0 +1,56 @@ +/** + * Shared input-validation helpers for AI tool action multiplexers. + * + * The `manage_*` tools accept a flat action + params object whose Zod layer + * (`toolInputSchemas`) marks every id/payload optional — required-ness depends + * on the action. Before this helper existed, a missing param sailed past that + * layer, got coerced (`String(undefined)` → the literal string "undefined"), + * and blew up downstream as a raw DB/uuid error the caller saw as an opaque + * HTTP 500 (#2362). These helpers turn both failure modes into the same + * structured tool error shape the service-error paths already use: + * `{ "error": "", "code": "VALIDATION_ERROR" }`. + */ + +import { ZodError } from 'zod'; + +/** Structured tool-error JSON for a validation failure. */ +export function validationErrorJson(message: string): string { + return JSON.stringify({ error: message, code: 'VALIDATION_ERROR' }); +} + +/** + * Presence check for the params an action requires, run BEFORE any coercion. + * Returns a structured error JSON naming the missing params, or null when all + * are present. `null` and `undefined` both count as missing; empty strings/ + * arrays/objects are left to the per-payload Zod schemas. + */ +export function missingParamsJson( + input: Record, + action: string, + required: readonly string[] +): string | null { + const missing = required.filter((key) => input[key] == null); + if (missing.length === 0) return null; + return validationErrorJson( + `Missing required parameter${missing.length > 1 ? 's' : ''} for action "${action}": ${missing.join(', ')}` + ); +} + +/** + * Convert a ZodError into a structured tool error carrying each issue's path + * (e.g. "line.sourceType: Invalid option ..."), or null for any other error so + * the caller can fall through to its domain service-error mapping / rethrow. + * Payload parses should wrap the value under its param name + * (`z.object({ line: schema }).parse({ line: input.line })`) so the paths are + * self-describing for the calling model. + */ +export function zodErrorToJson(err: unknown): string | null { + if (!(err instanceof ZodError)) return null; + const message = err.issues + .map((issue) => { + const path = issue.path.join('.'); + return path ? `${path}: ${issue.message}` : issue.message; + }) + .join('; '); + return validationErrorJson(message); +} diff --git a/apps/api/src/services/aiToolsBilling.manageInvoices.test.ts b/apps/api/src/services/aiToolsBilling.manageInvoices.test.ts index 8c0da6879d..b31247c767 100644 --- a/apps/api/src/services/aiToolsBilling.manageInvoices.test.ts +++ b/apps/api/src/services/aiToolsBilling.manageInvoices.test.ts @@ -276,6 +276,25 @@ describe('manage_invoices', () => { expect(JSON.parse(out)).toHaveProperty('error'); }); + + it('issue without invoiceId returns a structured VALIDATION_ERROR instead of coercing "undefined" (#2362 sweep)', async () => { + const out = await getTool().handler({ action: 'issue' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('invoiceId'); + expect(invoiceService.issueInvoice).not.toHaveBeenCalled(); + }); + + it('add_catalog_line without catalogItemId/quantity returns a structured VALIDATION_ERROR (#2362 sweep)', async () => { + const out = await getTool().handler({ action: 'add_catalog_line', invoiceId: 'inv-1' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('catalogItemId'); + expect(parsed.error).toContain('quantity'); + expect(invoiceService.addCatalogLine).not.toHaveBeenCalled(); + }); }); describe('get_invoice / list_invoices deposit fields', () => { diff --git a/apps/api/src/services/aiToolsBilling.ts b/apps/api/src/services/aiToolsBilling.ts index dca31dac27..48f9779fad 100644 --- a/apps/api/src/services/aiToolsBilling.ts +++ b/apps/api/src/services/aiToolsBilling.ts @@ -45,6 +45,7 @@ import { createInvoicePayLink } from './invoiceCheckout'; import { InvoiceServiceError, type InvoiceActor } from './invoiceTypes'; import { computeContractEstimate, getContract } from './contractService'; import { toCents } from './invoiceMath'; +import { missingParamsJson, zodErrorToJson } from './aiToolValidation'; type UpdateInvoiceLinePatch = Parameters[2]; type UpdateInvoiceHeaderPatch = Parameters[1]; @@ -82,6 +83,30 @@ function withDepositPaid= toCents(inv.depositDue) }; } +/** + * Params each manage_invoices action requires, presence-checked BEFORE any + * `String(...)` coercion so a missing id can't become the literal string + * "undefined" and die downstream as an opaque uuid/DB 500 (#2362 sweep). + */ +const MANAGE_INVOICES_REQUIRED: Record = { + create_draft: ['orgId'], + add_manual_line: ['invoiceId', 'line'], + add_catalog_line: ['invoiceId', 'catalogItemId', 'quantity'], + add_bundle_line: ['invoiceId', 'bundleId', 'quantity'], + add_contract_line: ['invoiceId', 'contractId', 'contractLineId'], + update_line: ['invoiceId', 'lineId', 'patch'], + remove_line: ['invoiceId', 'lineId'], + update_header: ['invoiceId', 'patch'], + delete_draft: ['invoiceId'], + assemble_from_org: ['orgId', 'from', 'to'], + assemble_from_ticket: ['ticketId'], + issue: ['invoiceId'], + void: ['invoiceId', 'reason'], + record_payment: ['invoiceId', 'payment'], + void_payment: ['paymentId'], + create_pay_link: ['invoiceId'], +}; + export function registerBillingTools(aiTools: Map): void { aiTools.set('list_invoices', { tier: 2 as AiToolTier, @@ -202,8 +227,16 @@ export function registerBillingTools(aiTools: Map): void { const actor = actorFromAuth(auth); const s = (k: string) => (input[k] == null ? undefined : String(input[k])); + const action = String(input.action); + const required = MANAGE_INVOICES_REQUIRED[action]; + if (!required) { + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); + } + const missing = missingParamsJson(input, action, required); + if (missing) return missing; + try { - switch (input.action) { + switch (action) { case 'create_draft': return JSON.stringify(await createManualInvoice( { @@ -271,10 +304,10 @@ export function registerBillingTools(aiTools: Map): void { case 'create_pay_link': return JSON.stringify(await createInvoicePayLink(String(input.invoiceId), actor)); default: - return JSON.stringify({ error: `Unknown action: ${String(input.action)}` }); + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); } } catch (err) { - const json = serviceErrorToJson(err); + const json = serviceErrorToJson(err) ?? zodErrorToJson(err); if (json) return json; throw err; } diff --git a/apps/api/src/services/aiToolsCatalog.manageCatalog.test.ts b/apps/api/src/services/aiToolsCatalog.manageCatalog.test.ts index 98613134a6..4bed1b850c 100644 --- a/apps/api/src/services/aiToolsCatalog.manageCatalog.test.ts +++ b/apps/api/src/services/aiToolsCatalog.manageCatalog.test.ts @@ -172,4 +172,13 @@ describe('manage_catalog', () => { expect(JSON.parse(out)).toHaveProperty('error'); }); + + it('update_item without catalogId returns a structured VALIDATION_ERROR instead of coercing "undefined" (#2362 sweep)', async () => { + const out = await getTool().handler({ action: 'update_item', item: { name: 'x' } }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('catalogId'); + expect(catalogService.updateCatalogItem).not.toHaveBeenCalled(); + }); }); diff --git a/apps/api/src/services/aiToolsCatalog.ts b/apps/api/src/services/aiToolsCatalog.ts index 80d1f4e0d9..8a64d7f45b 100644 --- a/apps/api/src/services/aiToolsCatalog.ts +++ b/apps/api/src/services/aiToolsCatalog.ts @@ -33,6 +33,21 @@ import { updateCatalogItem, type CatalogActor } from './catalogService'; +import { missingParamsJson, zodErrorToJson } from './aiToolValidation'; + +/** + * Params each manage_catalog action requires, presence-checked BEFORE any + * `String(...)` coercion so a missing id can't become the literal string + * "undefined" and die downstream as an opaque uuid/DB 500 (#2362 sweep). + */ +const MANAGE_CATALOG_REQUIRED: Record = { + create_item: ['item'], + update_item: ['catalogId', 'item'], + archive_item: ['catalogId'], + set_org_price: ['catalogId', 'orgId', 'override'], + remove_org_price: ['catalogId', 'orgId'], + set_bundle_components: ['catalogId', 'components'], +}; function actorFromAuth(auth: AuthContext): CatalogActor { return { @@ -195,10 +210,17 @@ export function registerCatalogTools(aiTools: Map): void { }, handler: async (input, auth) => { const actor = actorFromAuth(auth); - const s = (k: string) => (input[k] == null ? undefined : String(input[k])); + + const action = String(input.action); + const required = MANAGE_CATALOG_REQUIRED[action]; + if (!required) { + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); + } + const missing = missingParamsJson(input, action, required); + if (missing) return missing; try { - switch (input.action) { + switch (action) { case 'create_item': return JSON.stringify(await createCatalogItem(input.item as CreateCatalogItemInput, actor)); case 'update_item': @@ -229,10 +251,10 @@ export function registerCatalogTools(aiTools: Map): void { actor )); default: - return JSON.stringify({ error: `Unknown action: ${s('action')}` }); + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); } } catch (err) { - const json = serviceErrorToJson(err); + const json = serviceErrorToJson(err) ?? zodErrorToJson(err); if (json) return json; throw err; } diff --git a/apps/api/src/services/aiToolsContracts.manageContracts.test.ts b/apps/api/src/services/aiToolsContracts.manageContracts.test.ts index 88fae59ed7..ef1e5f5c7b 100644 --- a/apps/api/src/services/aiToolsContracts.manageContracts.test.ts +++ b/apps/api/src/services/aiToolsContracts.manageContracts.test.ts @@ -158,4 +158,13 @@ describe('manage_contracts', () => { expect(JSON.parse(out)).toHaveProperty('error'); }); + + it('activate without contractId returns a structured VALIDATION_ERROR instead of coercing "undefined" (#2362 sweep)', async () => { + const out = await getTool().handler({ action: 'activate' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('contractId'); + expect(contractService.activateContract).not.toHaveBeenCalled(); + }); }); diff --git a/apps/api/src/services/aiToolsContracts.ts b/apps/api/src/services/aiToolsContracts.ts index 2364cca89c..5c4052eb6b 100644 --- a/apps/api/src/services/aiToolsContracts.ts +++ b/apps/api/src/services/aiToolsContracts.ts @@ -35,6 +35,24 @@ import { cancelContract } from './contractService'; import { ContractServiceError, type ContractActor } from './contractTypes'; +import { missingParamsJson, zodErrorToJson } from './aiToolValidation'; + +/** + * Params each manage_contracts action requires, presence-checked BEFORE any + * `String(...)` coercion so a missing id can't become the literal string + * "undefined" and die downstream as an opaque uuid/DB 500 (#2362 sweep). + */ +const MANAGE_CONTRACTS_REQUIRED: Record = { + create_draft: ['input'], + update: ['contractId', 'patch'], + delete_draft: ['contractId'], + add_line: ['contractId', 'line'], + remove_line: ['contractId', 'lineId'], + activate: ['contractId'], + pause: ['contractId'], + resume: ['contractId'], + cancel: ['contractId'], +}; function actorFromAuth(auth: AuthContext): ContractActor { return { @@ -156,10 +174,17 @@ export function registerContractTools(aiTools: Map): void { }, handler: async (input, auth) => { const actor = actorFromAuth(auth); - const s = (k: string) => (input[k] == null ? undefined : String(input[k])); + + const action = String(input.action); + const required = MANAGE_CONTRACTS_REQUIRED[action]; + if (!required) { + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); + } + const missing = missingParamsJson(input, action, required); + if (missing) return missing; try { - switch (input.action) { + switch (action) { case 'create_draft': return JSON.stringify(await createContract(input.input as CreateContractInput, actor)); case 'update': @@ -189,10 +214,10 @@ export function registerContractTools(aiTools: Map): void { case 'cancel': return JSON.stringify(await cancelContract(String(input.contractId), actor)); default: - return JSON.stringify({ error: `Unknown action: ${s('action')}` }); + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); } } catch (err) { - const json = serviceErrorToJson(err); + const json = serviceErrorToJson(err) ?? zodErrorToJson(err); if (json) return json; throw err; } diff --git a/apps/api/src/services/aiToolsQuotes.test.ts b/apps/api/src/services/aiToolsQuotes.test.ts index ed380d630d..e1803aa6c4 100644 --- a/apps/api/src/services/aiToolsQuotes.test.ts +++ b/apps/api/src/services/aiToolsQuotes.test.ts @@ -70,6 +70,12 @@ const auth: AuthContext = { const actor = { userId: 'u-1', partnerId: 'p-1', accessibleOrgIds: ['org-1'] }; +// Payloads are parsed with the shared route schemas, which require real UUIDs. +const ORG_UUID = '11111111-1111-4111-8111-111111111111'; +const SITE_UUID = '22222222-2222-4222-8222-222222222222'; +const CATALOG_UUID = '33333333-3333-4333-8333-333333333333'; +const BLOCK_UUID = '44444444-4444-4444-8444-444444444444'; + function getTool(): AiTool { const map = new Map(); registerQuoteTools(map); @@ -83,8 +89,8 @@ describe('manage_quotes', () => { it('create_draft calls createQuote with input payload and actor built from auth', async () => { const input = { - orgId: 'org-1', - siteId: 'site-1', + orgId: ORG_UUID, + siteId: SITE_UUID, currencyCode: 'USD', introNotes: 'Proposal intro', }; @@ -167,9 +173,9 @@ describe('manage_quotes', () => { { action: 'add_catalog_line', quoteId: 'quote-1', - catalogItemId: 'catalog-1', + catalogItemId: CATALOG_UUID, quantity: 2, - blockId: 'block-1', + blockId: BLOCK_UUID, partNumber: 'MPN-42', }, auth, @@ -177,9 +183,9 @@ describe('manage_quotes', () => { expect(quoteService.addCatalogLine).toHaveBeenCalledWith( 'quote-1', - 'catalog-1', + CATALOG_UUID, 2, - 'block-1', + BLOCK_UUID, actor, { partNumber: 'MPN-42' }, ); @@ -217,3 +223,124 @@ describe('manage_quotes', () => { expect(JSON.parse(out)).toHaveProperty('error'); }); }); + +describe('manage_quotes input validation (#2362)', () => { + beforeEach(() => vi.clearAllMocks()); + + it('update without quoteId returns a structured VALIDATION_ERROR instead of throwing', async () => { + const out = await getTool().handler({ action: 'update', patch: {} }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('quoteId'); + expect(quoteService.updateQuote).not.toHaveBeenCalled(); + }); + + it('update without patch returns a structured VALIDATION_ERROR', async () => { + const out = await getTool().handler({ action: 'update', quoteId: 'quote-1' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('patch'); + expect(quoteService.updateQuote).not.toHaveBeenCalled(); + }); + + it('create_draft without input returns a structured VALIDATION_ERROR', async () => { + const out = await getTool().handler({ action: 'create_draft' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('input'); + expect(quoteService.createQuote).not.toHaveBeenCalled(); + }); + + it('add_manual_line missing sourceType/taxable returns a VALIDATION_ERROR naming the fields', async () => { + const out = await getTool().handler( + { + action: 'add_manual_line', + quoteId: 'quote-1', + line: { name: 'Onsite labor', description: 'Two hours', quantity: 2, unitPrice: 150 }, + }, + auth, + ); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('line.sourceType'); + expect(parsed.error).toContain('line.taxable'); + expect(quoteService.addManualLine).not.toHaveBeenCalled(); + }); + + it('add_manual_line with a valid line passes the parsed line (with schema defaults) to the service', async () => { + const out = await getTool().handler( + { + action: 'add_manual_line', + quoteId: 'quote-1', + line: { sourceType: 'manual', name: 'Onsite labor', quantity: 2, unitPrice: 150, taxable: false }, + }, + auth, + ); + + expect(quoteService.addManualLine).toHaveBeenCalledWith( + 'quote-1', + expect.objectContaining({ + sourceType: 'manual', + name: 'Onsite labor', + quantity: 2, + unitPrice: 150, + taxable: false, + // Defaults applied by quoteLineInputSchema + customerVisible: true, + recurrence: 'one_time', + depositEligible: false, + }), + actor, + ); + expect(JSON.parse(out)).toEqual({ id: 'line-1', quoteId: 'quote-1' }); + }); + + it('add_catalog_line with partNumber but no catalogItemId returns a VALIDATION_ERROR, not a throw', async () => { + const out = await getTool().handler( + { action: 'add_catalog_line', quoteId: 'quote-1', partNumber: 'MPN-42' }, + auth, + ); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('catalogItemId'); + expect(quoteService.addCatalogLine).not.toHaveBeenCalled(); + }); + + it('add_catalog_line with a non-UUID catalogItemId returns a VALIDATION_ERROR with the field path', async () => { + const out = await getTool().handler( + { action: 'add_catalog_line', quoteId: 'quote-1', catalogItemId: 'not-a-uuid', quantity: 1 }, + auth, + ); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('catalogItemId'); + expect(quoteService.addCatalogLine).not.toHaveBeenCalled(); + }); + + it('add_block with a mismatched content shape returns a VALIDATION_ERROR', async () => { + const out = await getTool().handler( + { action: 'add_block', quoteId: 'quote-1', block: { blockType: 'heading', content: {} } }, + auth, + ); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('block.'); + expect(quoteService.addBlock).not.toHaveBeenCalled(); + }); + + it('remove_line without lineId returns a structured VALIDATION_ERROR', async () => { + const out = await getTool().handler({ action: 'remove_line', quoteId: 'quote-1' }, auth); + + const parsed = JSON.parse(out); + expect(parsed.code).toBe('VALIDATION_ERROR'); + expect(parsed.error).toContain('lineId'); + expect(quoteService.removeLine).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/services/aiToolsQuotes.ts b/apps/api/src/services/aiToolsQuotes.ts index 4a5446a730..e57649603a 100644 --- a/apps/api/src/services/aiToolsQuotes.ts +++ b/apps/api/src/services/aiToolsQuotes.ts @@ -11,16 +11,30 @@ * which already enforce org access through `assertOrg`/`getQuote`. A thrown * `QuoteServiceError` (e.g. ORG_DENIED, QUOTE_NOT_FOUND, NOT_A_DRAFT) is * converted to a JSON error string rather than propagated. + * + * Input validation (#2362): the flat `toolInputSchemas.manage_quotes` layer + * marks every id/payload optional because required-ness depends on the action. + * The handler therefore (1) presence-checks each action's required params + * BEFORE any `String(...)` coercion, and (2) parses the `input`/`patch`/ + * `block`/`line` payloads with the same shared Zod schemas the HTTP routes + * use, so a malformed call returns a structured + * `{ error, code: 'VALIDATION_ERROR' }` instead of escaping as a raw 500. */ -import type { - CreateQuoteInput, - QuoteBlockInput, - QuoteLineInput, - UpdateQuoteInput, +import { z } from 'zod'; +import { + createQuoteSchema, + updateQuoteSchema, + quoteBlockInputSchema, + quoteLineInputSchema, + updateQuoteLineSchema, + catalogQuoteLineSchema, + reorderBlocksSchema, + reorderLinesSchema, } from '@breeze/shared'; import type { AuthContext } from '../middleware/auth'; import type { AiTool, AiToolTier } from './aiTools'; +import { missingParamsJson, zodErrorToJson } from './aiToolValidation'; import { createQuote, getQuote, @@ -61,6 +75,38 @@ function serviceErrorToJson(err: unknown): string | null { return null; } +/** + * Params each action requires. Checked before any coercion so a missing id can + * never become the literal string "undefined" (which used to reach the DB and + * die as an opaque uuid-parse 500 — #2362). + */ +const REQUIRED_PARAMS: Record = { + create_draft: ['input'], + update: ['quoteId', 'patch'], + delete_draft: ['quoteId'], + add_block: ['quoteId', 'block'], + update_block: ['quoteId', 'blockId', 'block'], + delete_block: ['quoteId', 'blockId'], + reorder_blocks: ['quoteId', 'blockIds'], + add_manual_line: ['quoteId', 'line'], + add_catalog_line: ['quoteId', 'catalogItemId', 'quantity'], + update_line: ['quoteId', 'lineId', 'patch'], + remove_line: ['quoteId', 'lineId'], + reorder_lines: ['quoteId', 'blockId', 'lineIds'], + send: ['quoteId'], + decline: ['quoteId'], + create_pay_link: ['quoteId'], +}; + +// Payload parsers wrap the value under its param name so ZodError paths are +// self-describing ("line.sourceType: ...", "input.orgId: ..."). These are the +// SAME schemas the HTTP quote routes validate with — one source of truth. +const createPayload = z.object({ input: createQuoteSchema }); +const headerPatchPayload = z.object({ patch: updateQuoteSchema }); +const blockPayload = z.object({ block: quoteBlockInputSchema }); +const linePayload = z.object({ line: quoteLineInputSchema }); +const linePatchPayload = z.object({ patch: updateQuoteLineSchema }); + export function registerQuoteTools(aiTools: Map): void { aiTools.set('manage_quotes', { tier: 2 as AiToolTier, @@ -69,7 +115,12 @@ export function registerQuoteTools(aiTools: Map): void { name: 'manage_quotes', description: 'Create and manage quotes/proposals for orgs the caller can access: draft header edits, blocks, lines, ' + - 'send/decline lifecycle actions, and accepted-quote pay links. Sending a quote requires approval.', + 'send/decline lifecycle actions, and accepted-quote pay links. Sending a quote requires approval. ' + + 'Required params per action — create_draft: input; update: quoteId, patch; delete_draft/send/decline/' + + 'create_pay_link: quoteId; add_block: quoteId, block; update_block: quoteId, blockId, block; delete_block: ' + + 'quoteId, blockId; reorder_blocks: quoteId, blockIds; add_manual_line: quoteId, line; add_catalog_line: ' + + 'quoteId, catalogItemId, quantity (blockId optional); update_line: quoteId, lineId, patch; remove_line: ' + + 'quoteId, lineId; reorder_lines: quoteId, blockId, lineIds.', input_schema: { type: 'object' as const, properties: { @@ -96,11 +147,27 @@ export function registerQuoteTools(aiTools: Map): void { quoteId: { type: 'string', description: 'Quote UUID' }, blockId: { type: 'string' }, lineId: { type: 'string' }, - catalogItemId: { type: 'string' }, - quantity: { type: 'number' }, - partNumber: { type: 'string' }, + catalogItemId: { + type: 'string', + description: + 'Catalog item UUID — REQUIRED for add_catalog_line. The item must be looked up by UUID ' + + '(use search_catalog); partNumber is NOT a lookup key.', + }, + quantity: { type: 'number', description: 'Line quantity (> 0) — required for add_catalog_line' }, + partNumber: { + type: 'string', + description: + 'Optional part-number override STORED on the created line (add_catalog_line only). ' + + 'Not a lookup key — the catalog item is always selected by catalogItemId.', + }, reason: { type: 'string', description: 'Decline reason' }, - input: { type: 'object', description: 'Full create-quote payload including orgId and siteId' }, + input: { + type: 'object', + description: + 'Create-quote payload (create_draft). Required: orgId (UUID). Optional: siteId (UUID), ' + + 'title, currencyCode (3-letter, default USD), expiryDate (YYYY-MM-DD), introNotes, terms, ' + + 'termsAndConditions.', + }, patch: { type: 'object', description: @@ -114,8 +181,24 @@ export function registerQuoteTools(aiTools: Map): void { depositEligible: { type: 'boolean' }, }, }, - block: { type: 'object', description: 'Quote block input fields' }, - line: { type: 'object', description: 'Manual quote line fields' }, + block: { + type: 'object', + description: + 'Quote block input (add_block/update_block). Required: blockType (\'heading\'|\'rich_text\'|\'image\'|' + + '\'line_items\') plus a matching content object — heading: {text, level? (1-3)}; rich_text: {html}; ' + + 'image: {imageId (quote image UUID), caption?, width?}; line_items: {label?}. update_block must ' + + 'restate the existing blockType (the type itself cannot change).', + }, + line: { + type: 'object', + description: + 'Manual quote line fields (add_manual_line). Required: sourceType (\'manual\'|\'catalog\'|\'bundle\' — ' + + 'use \'manual\' for a hand-entered line), quantity (> 0), unitPrice, taxable (boolean), and at least ' + + 'one of name/description. Optional: name, description, customerVisible (default true), recurrence ' + + '(\'one_time\'|\'monthly\'|\'annual\', default \'one_time\'), termMonths, billingFrequency ' + + '(\'monthly\'|\'annual\'), unitCost, sku, partNumber, depositEligible (default false), blockId (UUID), ' + + 'catalogItemId (UUID).', + }, blockIds: { type: 'array', items: { type: 'string' }, description: 'Ordered block UUIDs' }, lineIds: { type: 'array', items: { type: 'string' }, description: 'Ordered line UUIDs' }, }, @@ -126,13 +209,25 @@ export function registerQuoteTools(aiTools: Map): void { const actor = actorFromAuth(auth); const s = (k: string) => (input[k] == null ? undefined : String(input[k])); + const action = String(input.action); + const required = REQUIRED_PARAMS[action]; + if (!required) { + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); + } + const missing = missingParamsJson(input, action, required); + if (missing) return missing; + try { - switch (input.action) { + switch (action) { case 'create_draft': - return JSON.stringify(await createQuote(input.input as CreateQuoteInput, actor)); + return JSON.stringify(await createQuote( + createPayload.parse({ input: input.input }).input, + actor + )); case 'update': { const quoteId = String(input.quoteId); - await updateQuote(quoteId, input.patch as UpdateQuoteInput, actor); + const { patch } = headerPatchPayload.parse({ patch: input.patch }); + await updateQuote(quoteId, patch, actor); // Re-read rather than return updateQuote's raw row: the raw row carries // depositType/depositPercent/depositAmount but not the derived // depositDueTotal/categoryBreakdown (computed from current lines in @@ -147,50 +242,63 @@ export function registerQuoteTools(aiTools: Map): void { case 'add_block': return JSON.stringify(await addBlock( String(input.quoteId), - input.block as QuoteBlockInput, + blockPayload.parse({ block: input.block }).block, actor )); case 'update_block': return JSON.stringify(await updateBlock( String(input.quoteId), String(input.blockId), - input.block as QuoteBlockInput, + blockPayload.parse({ block: input.block }).block, actor )); case 'delete_block': await deleteBlock(String(input.quoteId), String(input.blockId), actor); return JSON.stringify({ ok: true }); - case 'reorder_blocks': - await reorderBlocks(String(input.quoteId), input.blockIds as string[], actor); + case 'reorder_blocks': { + const { blockIds } = reorderBlocksSchema.parse({ blockIds: input.blockIds }); + await reorderBlocks(String(input.quoteId), blockIds, actor); return JSON.stringify({ ok: true }); + } case 'add_manual_line': return JSON.stringify(await addManualLine( String(input.quoteId), - input.line as QuoteLineInput, + linePayload.parse({ line: input.line }).line, actor )); - case 'add_catalog_line': + case 'add_catalog_line': { + // Same schema the POST /:id/lines/catalog route validates with: + // guid catalogItemId + positive quantity, optional blockId/partNumber. + const args = catalogQuoteLineSchema.parse({ + catalogItemId: input.catalogItemId, + quantity: input.quantity, + blockId: input.blockId ?? undefined, + partNumber: input.partNumber == null ? undefined : String(input.partNumber), + }); return JSON.stringify(await addCatalogLine( String(input.quoteId), - String(input.catalogItemId), - Number(input.quantity), - s('blockId'), + args.catalogItemId, + args.quantity, + args.blockId, actor, - { partNumber: input.partNumber == null ? null : String(input.partNumber) } + { partNumber: args.partNumber ?? null } )); + } case 'update_line': return JSON.stringify(await updateLine( String(input.quoteId), String(input.lineId), - input.patch as UpdateQuoteLinePatch, + linePatchPayload.parse({ patch: input.patch }).patch as UpdateQuoteLinePatch, actor )); case 'remove_line': await removeLine(String(input.quoteId), String(input.lineId), actor); return JSON.stringify({ ok: true }); - case 'reorder_lines': - await reorderLines(String(input.quoteId), String(input.blockId), input.lineIds as string[], actor); + case 'reorder_lines': { + const { lineIds } = reorderLinesSchema.parse({ lineIds: input.lineIds }); + await reorderLines(String(input.quoteId), String(input.blockId), lineIds, actor); return JSON.stringify({ ok: true }); + } case 'send': return JSON.stringify(await sendQuote(String(input.quoteId), actor)); case 'decline': @@ -198,10 +306,10 @@ export function registerQuoteTools(aiTools: Map): void { case 'create_pay_link': return JSON.stringify(await createQuotePayLink(String(input.quoteId), actor)); default: - return JSON.stringify({ error: `Unknown action: ${s('action')}` }); + return JSON.stringify({ error: `Unknown action: ${action}`, code: 'VALIDATION_ERROR' }); } } catch (err) { - const json = serviceErrorToJson(err); + const json = serviceErrorToJson(err) ?? zodErrorToJson(err); if (json) return json; throw err; }