fix(ai-tools): structured VALIDATION_ERROR for manage_* billing-domain tools (#2362)

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": "<message>", "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<string, unknown>,
+  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);
+}

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', () => {

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<typeof updateLine>[2];
 type UpdateInvoiceHeaderPatch = Parameters<typeof updateInvoice>[1];
@@ -82,6 +83,30 @@ function withDepositPaid<T extends { depositDue?: string | null; amountPaid: str
   return { ...inv, depositPaid: toCents(inv.amountPaid) >= 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<string, readonly string[]> = {
+  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<string, AiTool>): void {
   aiTools.set('list_invoices', {
     tier: 2 as AiToolTier,
@@ -202,8 +227,16 @@ export function registerBillingTools(aiTools: Map<string, AiTool>): 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<string, AiTool>): 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;
       }

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();
+  });
 });

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<string, readonly string[]> = {
+  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<string, AiTool>): 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<string, AiTool>): 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;
       }

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();
+  });
 });

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<string, readonly string[]> = {
+  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<string, AiTool>): 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<string, AiTool>): 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;
       }