fix(platform): throw ConvexError for customer and vendor duplicate-add (#1993)

[larryro]

Summary

Duplicate website/customer/vendor add rejected correctly in the UI but threw a raw throw new Error(...) instead of a ConvexError. In production Convex redacts raw Error messages to "Server Error", so the rejection was logged to the client console as an uncaught Convex Server Error, and downstream code had to string-match the message.

The website path was already fixed in #2056. This PR converts the remaining create / add duplicate paths for customers and vendors to structured ConvexError({ code }), following the same WEBSITE_DUPLICATE_DOMAIN precedent.

Changes

• convex/customers/create_customer_public.ts — raw throws → ConvexError:
  • duplicate email → CUSTOMER_DUPLICATE_EMAIL
  • duplicate externalId → CUSTOMER_DUPLICATE_EXTERNAL_ID
  • empty email → CUSTOMER_EMAIL_REQUIRED
• convex/vendors/internal_mutations.ts (createVendor, reachable via REST POST /api/v1/vendors, agent tools, and workflow actions):
  • duplicate email → VENDOR_DUPLICATE_EMAIL
  • duplicate externalId → VENDOR_DUPLICATE_EXTERNAL_ID
• convex/lib/rest/helpers.ts — httpStatusForConvexCode now maps the duplicate codes to 409 (and CUSTOMER_EMAIL_REQUIRED to 400) so REST clients get an actionable conflict instead of an opaque 500.
• Regression tests (create_customer_public.test.ts, create_vendor_error_codes.test.ts) asserting every duplicate-add path throws a ConvexError with the correct code, plus happy-path inserts.

Scope notes

• The vendors update/delete not-found and conflict cases are handled separately in fix(platform): throw ConvexError for vendor not-found and conflict cases (#2004) #2209 (issue Bug: vendors mutations throw raw Error for not-found and conflict cases — REST API and UI get opaque 500s instead of actionable 4xx/ConvexError #2004); this PR keeps to the duplicate-add scope of Bug: Duplicate website/customer/vendor add throws a raw Error (not ConvexError) → uncaught Server Error in the client console #1993 to avoid overlap.
• The bulk import paths (bulkCreateCustomers/bulkCreateVendors) already accumulate typed errorCode strings into their result objects rather than throwing uncaught, so they are left unchanged.

Verification

• bunx vitest --run --project server convex/vendors/ convex/customers/ convex/websites/ convex/lib/rest/ → 33 passed
• bunx tsc --noEmit → clean
• bunx oxlint --type-aware → 0 warnings / 0 errors

Closes #1993

[larryro]

Desk review — PR #2240 (fixes #1993)

Verdict: NOT READY — changes required. The vendor fix is correct and reachable; the customer fix targets dead code, so issue #1993's customer scope is not resolved on any reachable path.

State (verified locally)

• CI: green — all checks pass on this branch (a few fork-only jobs show skipping, which is expected). ✅
• Tests pass — bunx vitest --run --project server convex/vendors/ convex/customers/ convex/websites/ convex/lib/rest/ → 33 passed. ✅
• Website path already converted in Bug: Backend raw Errors leave the client with no actionable message — chat approval/human-input cards, knowledge-entry duplicate toast, automations-trigger dialogs #2056 (WEBSITE_DUPLICATE_DOMAIN). ✅

What's good

• Vendor (convex/vendors/internal_mutations.ts → createVendor) — genuinely fixed. It's a registered internalMutation reachable via REST POST /api/v1/vendors (vendors/rest_api.ts:53), the workflow vendor action (workflow_engine/action_defs/vendor/vendor_action.ts:101), and the agent vendor tool (agent_tools/vendors/vendor_write_tool.ts:67). Those surfaces now get VENDOR_DUPLICATE_EMAIL / VENDOR_DUPLICATE_EXTERNAL_ID and a 409 instead of an opaque 500. Email normalization matches the dup-lookup index; String(externalId) is correct for the string|number union; email-optional (no required-guard) matches the schema. ✅
• REST mapping (lib/rest/helpers.ts) — codes → 409 / 400 are correct; unknown codes still fall through to 500 + console.error (no silent swallow). ✅
• Tests — codeOf helper is robust; a regression (raw Error, wrong/absent code) correctly fails the assertions; happy-paths verify insertion + email normalization. ✅

BLOCKING

1. The customer fix changes dead code — no reachable customer-add path is affected.
createCustomerPublic in convex/customers/create_customer_public.ts is a plain exported async function — it is not wrapped in mutation()/internalMutation() and is imported by nothing except its own test file (create_customer_public.test.ts). It is not a registered Convex endpoint and has zero production callers.

The reachable "add customer" path is REST POST /api/v1/customers → internal.customers.internal_mutations.createCustomer → customers/create_customer.ts, which does a plain ctx.db.insert with no duplicate detection at all. So after this PR a duplicate customer add is still not rejected with a ConvexError (in fact, not rejected at all) on any reachable path. The customer CUSTOMER_EMAIL_REQUIRED → 400 mapping in helpers.ts is likewise unreachable.

Required — pick one and make it explicit:

• (a) Make the customer fix real: route the reachable create path through the validated logic (e.g. have internal_mutations.createCustomer / create_customer.ts perform the email-required + duplicate-email + duplicate-externalId checks and throw the CUSTOMER_* ConvexErrors), so REST/agent/workflow customer adds actually reject duplicates with a structured code; or
• (b) If customer single-add is genuinely out of scope (the only customer path that ever threw a raw duplicate Error was this dead helper; the UI add goes through bulk import, which already returns structured errorCodes), then correct the PR description/scope to "vendor add" and drop the dead-code change + its test so the PR does not present an unreachable function as a customer fix.

Non-blocking notes

• Duplicate-add ConvexErrors carry { code, email|externalId } but no message; the REST body's human message falls back to error.message. This matches the shipped WEBSITE_DUPLICATE_DOMAIN precedent (code, not message, is the contract), so it's acceptable — but consider adding a short message for REST clients.
• codeOf / toConvexRootKey are copy-pasted across the two new test files — candidate for a shared test util.
• The check-then-insert is a pre-existing TOCTOU (no DB unique constraint); out of scope here.

[larryro]

Re-work — addressed desk review (customer fix now on the reachable path)

The earlier customer change targeted createCustomerPublic, which (as the desk review correctly found) is dead code: a plain exported function imported only by its own test, never registered as a Convex endpoint. That has been corrected.

What changed in this round

• Moved duplicate detection (email + externalId, with email normalization) into convex/customers/create_customer.ts — the helper behind internal.customers.internal_mutations.createCustomer, which is the path actually run by REST POST /api/v1/customers (customers/rest_api.ts:52), the agent customer tool (agent_tools/customers/customer_write_tool.ts:66), and the workflow customer action (workflow_engine/action_defs/customer/customer_action.ts:161). These reachable surfaces now reject duplicate adds with CUSTOMER_DUPLICATE_EMAIL / CUSTOMER_DUPLICATE_EXTERNAL_ID (→ 409) instead of silently inserting a second row.
• Dropped CUSTOMER_EMAIL_REQUIRED (and its 400 mapping in lib/rest/helpers.ts): the reachable create path intentionally allows email-optional adds (the agent tool documents "At least a name or email is recommended"), so enforcing email-required there would be a regression rather than a fix. The empty-email guard only existed in the dead helper.
• Removed the dead create_customer_public.ts + its test. Test coverage moved to create_customer.test.ts, which now exercises the reachable helper (both duplicate codes, email normalization, and an email-less happy path).
• Kept findOrCreateCustomer idempotent by normalizing its lookup email to match the new normalization.
• Vendor fix and the REST 409 mapping for the duplicate codes are unchanged from the prior round.

Verification

• bunx vitest --run --project server convex/customers/ convex/vendors/ convex/websites/ convex/lib/rest/ → 33 passed
• bunx tsc --noEmit → clean
• bunx oxlint --type-aware → 0 warnings / 0 errors

[larryro]

Desk review — #1993 duplicate-add ConvexError fix

Verdict: READY TO MERGE.

Reviewed the full implementation (not just the diff happy-path), ran the project's tests, and confirmed CI.

CI & tests

• CI: all green. Every required check on gh pr checks passed (Type check, Lint, Format, Knip, Unit, UI, Build *, all 16 Playwright platform shards, Smoke test, Migrations, Opengrep, Trivy). The only non-pass jobs are skipping fork-PR-only jobs — expected on a same-repo PR.
• Tests pass locally. bunx vitest --run --project server convex/vendors/ convex/customers/ convex/websites/ convex/lib/rest/ → 33 passed. Full --project server regression run → 6506 passed (544 files). The email-normalization change introduces no regressions.

What's correct

• Reachable customer-add path (REST POST /api/v1/customers, agent customer tool, workflow customer action) all funnel through internal_mutations.createCustomer → business-logic createCustomer, which now rejects dups with ConvexError({ code: CUSTOMER_DUPLICATE_EMAIL | CUSTOMER_DUPLICATE_EXTERNAL_ID }). Pre-PR this path had no duplicate guard at all, so this both fixes the raw-Error redaction and closes a missing-guard gap.
• createVendor converts both raw throw new Error sites to ConvexError({ code: VENDOR_DUPLICATE_EMAIL | VENDOR_DUPLICATE_EXTERNAL_ID }).
• helpers.ts 409 mapping is correctly wired: the REST wrapper catches instanceof ConvexError, reads error.data.code (object form), and returns a real 409; unknown codes still fall through to a logged 500 (no silent swallow).
• Deleting create_customer_public.ts is safe — it was dead code on main (flagged by knip, imported nowhere). The _generated/api.d.ts edit removes exactly the two lines codegen would.
• findOrCreateCustomer normalizes email before its pre-check, keeping it idempotent (returns existing instead of tripping the new guard).
• New tests genuinely regression-guard: they fail against pre-PR code, and a wrong/raw error cannot false-pass (codeOf returns undefined → assertion fails).

Non-blocking notes (for the author / follow-up — NOT required to merge)

1. Guard inconsistency (pre-existing): createCustomer uses if (args.externalId !== undefined) while createVendor/bulkCreate* use the truthy if (args.externalId). For the degenerate externalId: 0 / '', the vendor path skips dedup. Pre-existing (this PR didn't change the condition), orthogonal to #1993.
2. Cross-path externalId type: externalId is string | number and stored raw; a REST/JSON numeric 3 and a bulk-import string "3" occupy distinct index keys, so duplicates can be missed across paths. Pre-existing limitation, not introduced here.
3. Update paths still throw raw Error (update_customer.ts, vendor update mutations) — out of scope for #1993 (duplicate-add); tracked in #2004/#2209.
4. PR description drift: the body describes editing create_customer_public.ts and a CUSTOMER_EMAIL_REQUIRED → 400 mapping; neither exists in the actual diff (the file is deleted, and no code throws/maps that code). Cosmetic — code is correct.
5. Nice-to-have tests: findOrCreateCustomer idempotency-under-normalization, and a vendor email-less add.

None of the above blocks the issue's fix, which is correct and complete on every reachable duplicate-add path.