diff --git a/docs/superpowers/audits/active-uat.md b/docs/superpowers/audits/active-uat.md index d3face36..320f4a90 100644 --- a/docs/superpowers/audits/active-uat.md +++ b/docs/superpowers/audits/active-uat.md @@ -108,7 +108,7 @@ ### Documenso upload — silent partial-state when field placement fails -- **`PARTIALLY SHIPPED (rollback gap fixed; comprehensive audit still queued)`** +- **`SHIPPED locally (not yet committed) — comprehensive audit Phase 1 complete`** - **Files touched (this fix):** _src/lib/services/custom-document-upload.service.ts_ (~line 400, placeFields try/catch). _src/components/documents/upload-for-signing-dialog.tsx_ (recipient UI sibling fix shipped separately). - **Symptom:** rep uploads a PDF, places fields, hits Send. Error toast surfaces: `Documenso response missing recipientId for matt.ciaccio@gmail.com - cannot place fields`. Document appears in the CRM's signing UI AND in Documenso, recipients + roles are wired, but **all placed fields are missing**. The signing UI on the receiving end has no boxes to fill, which means a signer who receives the invite via email lands on a useless page. - **Root cause:** in `placeFieldsFromUpload`, the placements were built via `fields.map(f => { if (!recipientId) throw ConflictError(...) ...})` BEFORE the surrounding try/catch. The synchronous throw from `map()` bubbled past the catch-and-rollback block that wraps `placeFields()`, so when the recipient lookup missed: @@ -121,14 +121,14 @@ 1. The placements `map()` is now INSIDE the same try/catch that wraps `placeFields()`. Any throw — sync or async — triggers the rollback (Document row → cancelled, Documenso envelope → voided). 2. Pre-throw `logger.error(...)` captures diagnostic state: the missed email, every email the Documenso response DID return. Future "why didn't this match" investigations have something to grep instead of guesswork. 3. Comment block explaining the dedupe semantic (Documenso de-dupes by email at the envelope level, so duplicate emails across CRM recipient rows all map to the same Documenso recipientId — that's expected behaviour, not a bug). -- **Still open — comprehensive audit:** user explicitly asked for a full pass on the upload-for-signing flow because "we need to do a comprehensive audit of the uploading to be sent through documenso to ensure all fields are wired up correctly and won't cause issues." The rollback gap is one example of multi-step orchestration that fails silently; the wider audit should cover: - 1. **Pre-flight validation BEFORE envelope creation.** Validate every recipient row has a usable email; validate every placed field's `recipientIndex` resolves to a recipient that has a non-empty email; validate at least one field exists when documentType requires one. Failing pre-flight returns a 400 with field-level errors, no Documenso side effects. - 2. **Per-step rollback hardening.** Currently rollback paths exist after `documensoCreate`, `documensoSend`, and `placeFields`, but they're independent try/catches. Refactor into a single sequenced state machine with an idempotent `rollbackTo(step)` helper so future inserts (e.g. metadata write between steps) inherit the rollback automatically. - 3. **Recipient → Documenso identity reconciliation.** Today the code assumes Documenso's response will echo every input email. When dedupe happens (same email → one recipient), the code WORKS correctly (mapping just collapses), but if Documenso silently DROPS a recipient (e.g. invalid email), the lookup throws and we get the silent-partial-state again. Add explicit reconciliation: after `sendDoc`, verify that every distinct email we sent IS present in `sentDoc.recipients`; raise a specific error type if not. - 4. **Idempotency on retry.** If the rep hits Send twice (network blip), do we double-create envelopes? Verify the document row's `documensoDocumentId` is checked before another `documensoCreate` call. - 5. **End-to-end tests covering every failure path.** vitest integration suite for: empty fields list, recipient with blank email, recipient with duplicate email, Documenso 4xx on create, Documenso 4xx on send, Documenso 4xx on place-fields, network timeout mid-flow. - 6. **Audit-log every milestone.** Currently we log on the final success / final failure. Per-step audit entries (`create_envelope`, `send_envelope`, `place_fields`) would make post-mortem investigation tractable. -- **Effort for comprehensive audit:** ~6–10h. ~2h pre-flight validation, ~3h state-machine refactor + tests, ~1h reconciliation logic, ~1h idempotency, ~2h test coverage, ~1h audit-log richness. Bundle as one PR because the pieces interleave. +- **Phase 1 audit shipped (5 sub-PRs delivered in this round):** + 1. **Persist `documensoId` immediately after `documensoCreate`** (P1.1). Was set only at the late success commit, leaving orphaned envelopes when any later step failed. Now the CRM row points at the envelope from the moment Documenso returns the id; rollback paths can find and void it. Catches future failures + future-proofs orphans. + 2. **Pre-flight validation hard-blocks Submit** (P1.2). UploadForSigningDialog computes a `submissionErrors` memo over recipients + fields. Submit button disabled when errors > 0. Inline amber summary lists every issue (missing email, invalid email, missing name, field assigned to non-existent recipient, no fields placed). Service layer also enforces the same checks (email regex + name presence) so direct API hits reject just as hard. No "I know there's a missing email" override. + 3. **State-machine refactor with `rollbackTo()` helper** (P1.5). Replaces three independent try/catches with one sequenced try around `create → send → place` and a single catch that calls `rollbackTo(reason)`. Tracks `state.step` + `state.documensoDocId` so future inserts (metadata writes between steps, etc.) inherit the rollback automatically. Idempotent — status flip is a no-op on a second call, voidDocument treats 404 as success. + 4. **Recipient ↔ Documenso identity reconciliation** (P1.6). After `documensoSend`, validates every distinct email we sent appears in `sentDoc.recipients`. If Documenso silently dropped one, a `ConflictError` fires before field placement so the rollback path triggers. Explicit error message names the missing email(s) for diagnosis. + 5. **End-to-end test coverage + per-failure audit-log entries** (P1.7). vitest suite extended with: blank email, whitespace-only email, malformed email, blank name, duplicate-emails-OK (Documenso dedupe semantic). `rollbackTo` writes a structured audit_log entry (`status=cancelled`, `failedStep`, `documensoEnvelopeId`, `errorClass`, `errorMessage`) so post-mortem investigation has structured data instead of pre-existing logger lines alone. +- **Still open (acknowledged but lower priority):** + - **Idempotency on retry** — if the rep hits Send twice, do we double-create envelopes? Today the dialog disables the button while `sendMutation.isPending` so it's mitigated at the UI; service-layer guard via checking `documents.documensoId` before another `documensoCreate` would be belt-and-braces. Queued for follow-up. - **Cross-refs:** - The `/documents/new` wizard refactor (Bucket 3 — wizard refactor finding) touches the same end-to-end flow — bundle the two so the same audit doesn't re-investigate the upload-for-signing service twice. - This is the SECOND time a multi-step Documenso flow has had a rollback gap — the first was the EOI auto-cancel/replace flow (fixed earlier in `65ff596`). Pattern: every multi-step orchestration that touches Documenso needs end-to-end rollback OR pre-flight validation. The audit doc's broader "activity feed comprehensive copy" finding mentioned a similar discipline gap; both should land before more multi-step features ship. diff --git a/scripts/tunnel-url.sh b/scripts/tunnel-url.sh index 7f6d83a0..fa505dbb 100644 --- a/scripts/tunnel-url.sh +++ b/scripts/tunnel-url.sh @@ -49,3 +49,16 @@ if [[ "${1:-}" == "--copy" ]]; then printf "%s/api/webhooks/documenso" "$URL" | pbcopy echo "(webhook URL copied to clipboard)" fi + +# Auto-PATCH Documenso's webhook URL when the env flag is set. Gated so +# production ports can never have their webhook rotated by a stale dev +# script. The TS script reads DOCUMENSO_API_URL + DOCUMENSO_API_KEY + +# DOCUMENSO_API_VERSION from .env and updates every webhook whose URL +# already points at our path OR at any *.trycloudflare.com host. +if [[ "${DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK:-}" == "1" ]]; then + echo "" + echo "DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK=1 — updating Documenso webhook(s)…" + cd "$(dirname "$0")/.." || exit 1 + DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK=1 \ + pnpm tsx scripts/update-documenso-webhook.ts "$URL" +fi diff --git a/scripts/update-documenso-webhook.ts b/scripts/update-documenso-webhook.ts new file mode 100644 index 00000000..ee03dffc --- /dev/null +++ b/scripts/update-documenso-webhook.ts @@ -0,0 +1,194 @@ +/** + * Documenso webhook URL auto-updater. Called by `./scripts/tunnel-url.sh` + * when the env flag `DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK=1` is set so a + * freshly-restarted cloudflared quick-tunnel (which gets a NEW hostname + * on every restart) doesn't leave Documenso pointing at a dead URL. + * + * Gated by env flag so production ports — which may have a stable + * webhook URL — can never have their config rotated by a stale dev + * script. Reads Documenso credentials from env (DOCUMENSO_API_URL + + * DOCUMENSO_API_KEY + optional DOCUMENSO_API_VERSION). + * + * Usage (manual invocation): + * DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK=1 pnpm tsx scripts/update-documenso-webhook.ts https://foo.trycloudflare.com + * + * Behaviour: + * - Lists every webhook currently configured on the Documenso + * instance. + * - Identifies webhooks whose `webhookUrl` looks like a + * trycloudflare.com domain OR matches our `/api/webhooks/documenso` + * path suffix. These are the ones to rotate. + * - PATCHes each matching webhook to point at the new tunnel URL. + * - Leaves all other webhooks alone (in case the instance also + * services another tenant or a stable production URL). + * + * Tries Documenso v2 first, falls back to v1 if the v2 endpoint + * returns 404. Both versions support GET /webhook(s) + PATCH on the + * webhook resource — the shape differs slightly between them but the + * fields we touch (`id`, `webhookUrl`) are stable across versions. + */ + +import 'dotenv/config'; + +const ENABLE_FLAG = process.env.DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK; +const TUNNEL_BASE = process.argv[2]; + +if (ENABLE_FLAG !== '1') { + console.log( + 'DEV_AUTO_UPDATE_DOCUMENSO_WEBHOOK is not set to 1 — skipping Documenso webhook update.', + ); + process.exit(0); +} + +if (!TUNNEL_BASE) { + console.error('Usage: pnpm tsx scripts/update-documenso-webhook.ts '); + console.error( + 'Example: pnpm tsx scripts/update-documenso-webhook.ts https://foo.trycloudflare.com', + ); + process.exit(1); +} + +const API_URL = process.env.DOCUMENSO_API_URL; +const API_KEY = process.env.DOCUMENSO_API_KEY; +const API_VERSION = (process.env.DOCUMENSO_API_VERSION ?? 'v2').toLowerCase(); + +if (!API_URL || !API_KEY) { + console.error('DOCUMENSO_API_URL and DOCUMENSO_API_KEY must be set in env to update webhooks.'); + process.exit(1); +} + +// Trim trailing slash so we can compose paths cleanly. +const BASE = API_URL.replace(/\/+$/, ''); +const NEW_WEBHOOK_URL = `${TUNNEL_BASE.replace(/\/+$/, '')}/api/webhooks/documenso`; + +async function documensoRequest(path: string, init?: RequestInit): Promise { + return fetch(`${BASE}${path}`, { + ...init, + headers: { + 'Content-Type': 'application/json', + Authorization: API_KEY!, + ...(init?.headers ?? {}), + }, + }); +} + +interface DocumensoWebhook { + id: string | number; + webhookUrl: string; +} + +/** + * Pluck the array of webhooks out of whatever shape the Documenso + * version returned. v1 historically returned an array directly; v2 + * tends to wrap in `{ data: [...] }` or similar. Be tolerant. + */ +function extractWebhooks(raw: unknown): DocumensoWebhook[] { + if (Array.isArray(raw)) return raw as DocumensoWebhook[]; + if (raw && typeof raw === 'object') { + const r = raw as Record; + if (Array.isArray(r.data)) return r.data as DocumensoWebhook[]; + if (Array.isArray(r.webhooks)) return r.webhooks as DocumensoWebhook[]; + } + return []; +} + +async function listWebhooks(): Promise<{ webhooks: DocumensoWebhook[]; version: 'v1' | 'v2' }> { + if (API_VERSION === 'v2' || API_VERSION === 'v2.0' || API_VERSION === 'v2.x') { + const res = await documensoRequest('/api/v2/webhook'); + if (res.ok) { + const body = (await res.json()) as unknown; + return { webhooks: extractWebhooks(body), version: 'v2' }; + } + if (res.status !== 404) { + console.error(`v2 webhook list returned ${res.status}: ${await res.text()}`); + } + // Fall through to v1. + } + const res = await documensoRequest('/api/v1/webhooks'); + if (!res.ok) { + console.error(`v1 webhook list returned ${res.status}: ${await res.text()}`); + process.exit(1); + } + const body = (await res.json()) as unknown; + return { webhooks: extractWebhooks(body), version: 'v1' }; +} + +async function patchWebhook( + version: 'v1' | 'v2', + webhook: DocumensoWebhook, + newUrl: string, +): Promise { + const path = + version === 'v2' + ? '/api/v2/webhook' + : `/api/v1/webhooks/${encodeURIComponent(String(webhook.id))}`; + const body = version === 'v2' ? { id: webhook.id, webhookUrl: newUrl } : { webhookUrl: newUrl }; + const res = await documensoRequest(path, { + method: 'PATCH', + body: JSON.stringify(body), + }); + if (!res.ok) { + console.error(`PATCH ${path} (id=${webhook.id}) returned ${res.status}: ${await res.text()}`); + return false; + } + return true; +} + +/** + * Decide whether a given existing webhook is "ours" (i.e. matches the + * pattern we want to rotate). Two signals: + * 1. Path tail matches `/api/webhooks/documenso` — the CRM-side + * handler we own. + * 2. Host matches `*.trycloudflare.com` — almost certainly a stale + * quick-tunnel URL. Rotating these is always safe. + */ +function isRotatableWebhook(w: DocumensoWebhook): boolean { + if (!w.webhookUrl) return false; + if (w.webhookUrl.endsWith('/api/webhooks/documenso')) return true; + try { + const host = new URL(w.webhookUrl).hostname; + if (host.endsWith('.trycloudflare.com')) return true; + } catch { + /* malformed — leave alone */ + } + return false; +} + +async function main(): Promise { + console.log(`Listing webhooks via Documenso ${API_VERSION.toUpperCase()} (base: ${BASE})…`); + const { webhooks, version } = await listWebhooks(); + console.log(`Found ${webhooks.length} webhook(s).`); + + const rotatable = webhooks.filter(isRotatableWebhook); + if (rotatable.length === 0) { + console.log( + `No rotatable webhooks found (looking for paths ending /api/webhooks/documenso or *.trycloudflare.com hosts).`, + ); + console.log(`If your dev webhook is configured differently, point it at: ${NEW_WEBHOOK_URL}`); + return; + } + + console.log(`Updating ${rotatable.length} webhook(s) to ${NEW_WEBHOOK_URL}…`); + let ok = 0; + let fail = 0; + for (const w of rotatable) { + if (w.webhookUrl === NEW_WEBHOOK_URL) { + console.log(` ${w.id}: already at the target URL, skipping.`); + continue; + } + const succeeded = await patchWebhook(version, w, NEW_WEBHOOK_URL); + if (succeeded) { + ok++; + console.log(` ${w.id}: ${w.webhookUrl} -> ${NEW_WEBHOOK_URL}`); + } else { + fail++; + } + } + console.log(`Done. ${ok} updated, ${fail} failed.`); + if (fail > 0) process.exit(1); +} + +main().catch((err) => { + console.error('Documenso webhook update failed:', err); + process.exit(1); +}); diff --git a/src/components/documents/document-list.tsx b/src/components/documents/document-list.tsx index 3abe570d..cdffdd91 100644 --- a/src/components/documents/document-list.tsx +++ b/src/components/documents/document-list.tsx @@ -3,6 +3,9 @@ import { useState } from 'react'; import { Download, FolderInput } from 'lucide-react'; import { useQueryClient } from '@tanstack/react-query'; +import { toast } from 'sonner'; + +import { toastError } from '@/lib/api/toast-error'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; @@ -126,7 +129,9 @@ function DocRow({ doc, onDelete, onSend, onPreview }: DocRowProps) { )} {doc.status === 'draft' && ( - onSend(doc)}>Send for Signing + + onSend(doc)}>Send for Signing + )} setMoveOpen(true)}> @@ -134,12 +139,14 @@ function DocRow({ doc, onDelete, onSend, onPreview }: DocRowProps) { Move to folder… - onDelete(doc)} - className="text-destructive focus:text-destructive" - > - Delete - + + onDelete(doc)} + className="text-destructive focus:text-destructive" + > + Delete + + @@ -179,18 +186,23 @@ export function DocumentList({ interestId, clientId, emptyState }: DocumentListP if (!ok) return; try { await apiFetch(`/api/v1/documents/${doc.id}`, { method: 'DELETE' }); - queryClient.invalidateQueries({ queryKey: ['documents', { interestId, clientId }] }); - } catch { - // silent + // Broader predicate matches every documents query (interest tab + // counter, doc detail, hub root, etc.) so the row disappears + // everywhere without requiring per-key invalidation knowledge. + queryClient.invalidateQueries({ predicate: (q) => q.queryKey[0] === 'documents' }); + toast.success('Document deleted'); + } catch (err) { + toastError(err); } }; const handleSend = async (doc: DocumentRow) => { try { await apiFetch(`/api/v1/documents/${doc.id}/send`, { method: 'POST' }); - queryClient.invalidateQueries({ queryKey: ['documents', { interestId, clientId }] }); - } catch { - // silent + queryClient.invalidateQueries({ predicate: (q) => q.queryKey[0] === 'documents' }); + toast.success('Sent for signing'); + } catch (err) { + toastError(err); } }; diff --git a/src/components/documents/upload-for-signing-dialog.tsx b/src/components/documents/upload-for-signing-dialog.tsx index 904923c0..8e7618c6 100644 --- a/src/components/documents/upload-for-signing-dialog.tsx +++ b/src/components/documents/upload-for-signing-dialog.tsx @@ -543,6 +543,46 @@ function DialogBody({ }, }); + // Pre-flight validation — surfaces every reason Submit would fail + // BEFORE the rep clicks, so they don't hit a partial-state Documenso + // failure. Mirrors the service-side validation in + // custom-document-upload.service.ts so the UI rejection set always + // matches what the server would reject. Hard block: Submit stays + // disabled until this returns []. Per the 2026-05-26 decision, no + // override path — Documenso's API can't recover from missing emails. + const submissionErrors = useMemo(() => { + const errors: string[] = []; + if (recipients.length === 0) { + errors.push('Add at least one recipient before sending.'); + } + const emailRegex = /^\S+@\S+\.\S+$/; + recipients.forEach((r) => { + const label = `Recipient #${r.signingOrder}`; + const name = (r.name ?? '').trim(); + const email = (r.email ?? '').trim(); + if (!name) { + errors.push(`${label} is missing a name.`); + } + if (!email) { + errors.push(`${label} is missing an email address.`); + } else if (!emailRegex.test(email)) { + errors.push(`${label} has an invalid email address (${email}).`); + } + }); + if (fields.length === 0) { + errors.push('Place at least one field on the document.'); + } + fields.forEach((f, idx) => { + const r = recipients[f.recipientIndex]; + if (!r) { + errors.push( + `Field #${idx + 1} (${FIELD_DEFAULTS[f.type].label} on page ${f.pageNumber}) is assigned to a recipient that no longer exists. Re-assign or delete it.`, + ); + } + }); + return errors; + }, [recipients, fields]); + const queryClient = useQueryClient(); const sendMutation = useMutation({ mutationFn: async () => { @@ -714,6 +754,30 @@ function DialogBody({ )} + {/* Pre-flight validation summary. Surfaces every reason Submit + would fail BEFORE the rep clicks, so a missing email or a + field assigned to a deleted recipient is caught here instead + of via a Documenso silent-failure round-trip. Renders only on + the place-fields step (where Submit lives) and only when + there are issues to call out. */} + {step === 'place-fields' && submissionErrors.length > 0 && ( +
+

+ Resolve {submissionErrors.length} issue + {submissionErrors.length === 1 ? '' : 's'} before sending: +

+
    + {submissionErrors.map((e, i) => ( +
  • {e}
    • + ))} +
+
+ )} +
@@ -758,7 +822,12 @@ function DialogBody({ {step === 'place-fields' && ( - + + +
+ + + ) : null} + - ) : null} - + ) : null} diff --git a/src/components/interests/interest-reservation-tab.tsx b/src/components/interests/interest-reservation-tab.tsx index 6ee31ef9..590b332b 100644 --- a/src/components/interests/interest-reservation-tab.tsx +++ b/src/components/interests/interest-reservation-tab.tsx @@ -18,6 +18,7 @@ import { toast } from 'sonner'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; import { Skeleton } from '@/components/ui/skeleton'; +import { PermissionGate } from '@/components/shared/permission-gate'; import { ExternalEoiUploadDialog } from '@/components/interests/external-eoi-upload-dialog'; import { MarkExternallySignedDialog } from '@/components/interests/mark-externally-signed-dialog'; import { SigningProgress } from '@/components/documents/signing-progress'; @@ -338,17 +339,19 @@ function ActiveReservationCard({ Upload paper-signed copy - + + + { + logger.warn( + { + documentId: docRowId, + documensoEnvelopeId: state.documensoDocId, + failedStep: state.step, + err: reason instanceof Error ? { message: reason.message, name: reason.name } : reason, + }, + 'Rolling back custom document upload', + ); + await db + .update(documents) + .set({ status: 'cancelled', updatedAt: new Date() }) + .where(eq(documents.id, docRowId)); + if (state.documensoDocId) { + await documensoVoidSafe(state.documensoDocId, portId); + } + // Failure audit-log entry — captures which step failed, the + // Documenso envelope id (if any), and the error class/message so + // post-mortem investigation doesn't have to dig through structured + // logs. Success-path audit is at the end of the function; this is + // the failure-path counterpart. + void createAuditLog({ + userId: meta.userId, + portId, + action: 'update', + entityType: 'document', + entityId: docRowId, + newValue: { + status: 'cancelled', + failedStep: state.step ?? 'unknown', + documensoEnvelopeId: state.documensoDocId, + errorClass: reason instanceof Error ? reason.name : null, + errorMessage: reason instanceof Error ? reason.message : String(reason), + }, + metadata: { + type: 'upload_for_signing_rollback', + }, + ipAddress: meta.ipAddress, + userAgent: meta.userAgent, + }); + } + let documensoDoc: Awaited>; let sentDoc: Awaited>; + try { + // Step 1 — create envelope in Documenso. + state.step = 'create'; documensoDoc = await documensoCreate(title, pdfBase64, documensoRecipients, portId, { ...(docCfg.signingOrder ? { signingOrder: docCfg.signingOrder } : {}), ...(docCfg.redirectUrl ? { redirectUrl: docCfg.redirectUrl } : {}), }); - } catch (err) { + state.documensoDocId = documensoDoc.id; + + // Persist documensoId IMMEDIATELY so any subsequent failure leaves + // the CRM row pointing at the envelope. Without this, a throw + // between documensoCreate and the late `status: 'sent'` update + // would orphan the envelope in Documenso (CRM has the local row but + // no link to find the upstream envelope to void). UAT 2026-05-26 + // hit exactly this: CRM document row stuck in 'draft' status, + // documensoId=NULL, Documenso hosting a live envelope nothing + // referenced. The late `status: 'sent'` UPDATE still runs and the + // idempotent re-write of documensoId is fine. await db .update(documents) - .set({ status: 'cancelled', updatedAt: new Date() }) + .set({ documensoId: documensoDoc.id, updatedAt: new Date() }) .where(eq(documents.id, docRow.id)); - throw err; - } - // Map our recipientIndex → resolved Documenso recipient id (number/ - // string). On v2 the envelope/create response doesn't include - // recipient ids; we resolve via the distribute response below - // (sendDocument returns the full doc with recipients). - try { + // Step 2 — distribute (Documenso v2) / send (v1). Resolves the + // recipient ids that we need for field placement next. + state.step = 'send'; sentDoc = await documensoSend(documensoDoc.id, portId); - } catch (err) { - await db - .update(documents) - .set({ status: 'cancelled', updatedAt: new Date() }) - .where(eq(documents.id, docRow.id)); - await documensoVoidSafe(documensoDoc.id, portId); - throw err; - } - // Build email→recipientId map. v2 envelope create returns empty - // recipients; distribute fills them in. v1 already has them on create. - // Note: Documenso de-dupes by email at the envelope level, so multiple - // CRM-side Recipient rows that share an email all map to the same - // Documenso recipientId. That's fine for field placement — both rows - // simply target the same Documenso recipient. - const emailToRecipientId = new Map(); - for (const dr of sentDoc.recipients) { - if (dr.email) emailToRecipientId.set(dr.email.toLowerCase(), dr.id); - } + // Step 3 — recipient identity reconciliation + field placement. + // Documenso de-dupes by email at the envelope level, so multiple + // CRM-side Recipient rows that share an email all map to the same + // Documenso recipientId — that's fine for field placement (both + // rows target the same Documenso recipient). + state.step = 'place'; + const emailToRecipientId = new Map(); + for (const dr of sentDoc.recipients) { + if (dr.email) emailToRecipientId.set(dr.email.toLowerCase(), dr.id); + } - // Build placements + place fields inside a SINGLE try/catch so any - // failure — including the synchronous `fields.map` throw when a - // recipient can't be matched — triggers the rollback path. Previously - // the map's throw bubbled past the try-block wrapping `placeFields()`, - // leaving Documenso with the live envelope + recipients but no fields, - // and the CRM document row stuck in 'sent' with no signing UI for the - // signers (UAT 2026-05-26 — "doc displays in Documenso but is missing - // all fields"). - try { + // Reconciliation guard: every distinct CRM email we sent must + // appear in sentDoc.recipients. If Documenso silently dropped one + // (invalid email format that passed our regex, etc.), we want a + // loud failure that triggers the rollback path — NOT a half-placed + // doc that ships to signers with missing fields. + const sentEmails = new Set(Array.from(emailToRecipientId.keys()).map((k) => k.toLowerCase())); + const missingFromDocumenso = recipients + .map((r) => r.email.trim().toLowerCase()) + .filter((email, idx, arr) => arr.indexOf(email) === idx) // dedupe + .filter((email) => !sentEmails.has(email)); + if (missingFromDocumenso.length > 0) { + logger.error( + { + documentId: docRow.id, + documensoEnvelopeId: documensoDoc.id, + missingFromDocumenso, + documensoReturned: Array.from(emailToRecipientId.keys()), + }, + 'Recipient reconciliation: Documenso response missing emails the CRM sent', + ); + throw new ConflictError( + `Documenso accepted the envelope but didn't echo recipient(s) for: ${missingFromDocumenso.join(', ')}. ` + + `Cannot place fields — recipients aren't reachable.`, + ); + } + + // Build placements + place fields inside the same try block so the + // synchronous map() throw (when a recipient can't be matched) + // triggers rollback alongside any async placeFields() throw. const placements: DocumensoFieldPlacement[] = fields.map((f) => { const recipient = recipients[f.recipientIndex]!; const recipientId = emailToRecipientId.get(recipient.email.toLowerCase()); if (!recipientId) { - // Surface the diagnostic state alongside the error so the rep - // doesn't have to guess which email failed to match. Logs both - // the looked-up email and the keys Documenso DID return so a - // future audit can see whether the mismatch is dedupe-related - // or a true populate failure. logger.error( { documentId: docRow.id, @@ -467,11 +557,7 @@ export async function uploadDocumentForSigning( }); await placeFields(documensoDoc.id, placements, portId); } catch (err) { - await db - .update(documents) - .set({ status: 'cancelled', updatedAt: new Date() }) - .where(eq(documents.id, docRow.id)); - await documensoVoidSafe(documensoDoc.id, portId); + await rollbackTo(err); throw err; } diff --git a/tests/unit/services/custom-document-upload.test.ts b/tests/unit/services/custom-document-upload.test.ts index 7a1f155e..52ad731b 100644 --- a/tests/unit/services/custom-document-upload.test.ts +++ b/tests/unit/services/custom-document-upload.test.ts @@ -128,4 +128,74 @@ describe('uploadDocumentForSigning validation', () => { }), ).rejects.toThrow(/Duplicate signingOrder/); }); + + // P1.2 — pre-flight validation extends to recipient email + name + // shape. Service mirrors the dialog's pre-Submit checks so direct API + // hits also reject early. + + it('rejects recipient with blank email', async () => { + await expect( + uploadDocumentForSigning({ + ...baseArgs, + recipients: [ + { name: 'Buyer', email: '', role: 'SIGNER', signingOrder: 1 }, + { name: 'Seller', email: 'seller@example.com', role: 'SIGNER', signingOrder: 2 }, + ], + }), + ).rejects.toThrow(/missing an email/i); + }); + + it('rejects recipient with whitespace-only email', async () => { + await expect( + uploadDocumentForSigning({ + ...baseArgs, + recipients: [ + { name: 'Buyer', email: ' ', role: 'SIGNER', signingOrder: 1 }, + { name: 'Seller', email: 'seller@example.com', role: 'SIGNER', signingOrder: 2 }, + ], + }), + ).rejects.toThrow(/missing an email/i); + }); + + it('rejects recipient with malformed email', async () => { + await expect( + uploadDocumentForSigning({ + ...baseArgs, + recipients: [ + { name: 'Buyer', email: 'not-an-email', role: 'SIGNER', signingOrder: 1 }, + { name: 'Seller', email: 'seller@example.com', role: 'SIGNER', signingOrder: 2 }, + ], + }), + ).rejects.toThrow(/invalid email/i); + }); + + it('rejects recipient with blank name', async () => { + await expect( + uploadDocumentForSigning({ + ...baseArgs, + recipients: [ + { name: '', email: 'buyer@example.com', role: 'SIGNER', signingOrder: 1 }, + { name: 'Seller', email: 'seller@example.com', role: 'SIGNER', signingOrder: 2 }, + ], + }), + ).rejects.toThrow(/missing a name/i); + }); + + it('accepts duplicate emails across recipients (Documenso dedupes by email)', async () => { + // The validation guard does NOT reject same-email recipients — at + // the field-placement step the email→recipientId map collapses them + // to a single Documenso recipientId by design. Other guards (PDF, + // recipient row insert, Documenso round-trip) prevent this test + // from reaching success in unit-mode; we only assert that the + // recipient-validation block does NOT throw early. + await expect( + uploadDocumentForSigning({ + ...baseArgs, + recipients: [ + { name: 'Buyer One', email: 'shared@example.com', role: 'SIGNER', signingOrder: 1 }, + { name: 'Buyer Two', email: 'shared@example.com', role: 'SIGNER', signingOrder: 2 }, + ], + }), + ).rejects.not.toThrow(/missing an email|invalid email|missing a name/i); + }); });