Audit cleanup completion plan, all tiers shipped: Tier 1 (security + data integrity) - A.7 RTBF true wipe: redact email_messages body/subject/addresses for threads owned by deleted client; redact document_sends.recipient_email; collect file storage keys + delete blobs post-commit. - A.8 user_permission_overrides FK: documented inline why cascade is correct (not set-null as audit suggested) — overrides have no value without their user. - W2.14 PII redaction: camelCase normalization in audit.ts + error-events.service.ts isSensitiveKey; added city/postal/country/ birth fragments. firstName/lastName/dateOfBirth/postalCode etc. now caught in BOTH masker paths. 12 new test cases lock the coverage. Tier 2 (Documenso completion + refactor) - C.2: documentEvents.recipient_email column + partial unique index for per-recipient webhook dedup (migration 0075). handleDocumentSigned now sets recipient_email on insert. - Phase 2: completion_cc_emails distribution. handleDocumentCompleted reads documents.completionCcEmails, filters out signer-duplicates case-insensitively, fans signed PDF out to non-signer recipients. - C.4: extracted createPublicInterest() service from the 346-line api/public/interests route. Route becomes a thin shell (rate-limit, port resolution, audit log, email fan-out). The trio creation logic is now unit-testable without an HTTP fixture. - Phase 4: POST /api/v1/document-templates/[id]/detect-fields wired to document-field-detector.detectFields(). Sparkles "Auto-detect" button added to template-editor.tsx — maps DetectedField → marker with best-guess merge token (DATE / NAME / EMAIL); user retags. Tier 3 (reporting + recommender snapshot lockfiles) - W7.reports: extracted rollupStageRevenue / rollupStageCounts / computeTotalForecast / computeOccupancyRate / rollupBerthStatusCounts into src/lib/services/report-math.ts (pure functions). 16 new tests including an inline-snapshot lockfile on a representative 7-stage forecast. report-generators.ts now delegates. - W7.recommender: 18 new toMatchSnapshot tripwires on classifyTier boundaries + computeHeat at canonical input points. Tier 4 (rolling) - W6.attach: fixed outdated CLAUDE.md claim — threshold banner is informational and never depended on IMAP; bounce monitoring (the IMAP poller) is separate. - D.1 + D.2: documented deferral inline with full why-not-build-it reasoning so a future engineer sees the rationale. - G.1: representative formatDate sweep (audit-log-list, user-list, document-templates merge tokens, document-signing email). Rest of the ~100 sites stay rolling. Quality gates: 1420/1420 vitest (46 new tests above baseline of 1374), tsc clean, 0 lint errors. Plan: docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md Migration: 0075_c2_document_events_recipient_email.sql (applied to dev DB). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1236 lines
58 KiB
Markdown
1236 lines
58 KiB
Markdown
# Audit Cleanup Completion — Master Plan (2026-05-18, Part 2)
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Close out everything outstanding in `docs/BACKLOG.md` sections A, C, D, E, G, H over a sequenced ~4-week sweep, ending with the backlog drained down to "no concrete trigger" parked items only.
|
||
|
||
**Architecture:** Six sequenced phases. Phase A is stop-ship priority (security + data integrity). Phase B layers in HIGH security + observability. Phase C re-opens the back-burnered Documenso build (phases 2-7 + Section C deferred items bundled). Phase D is mechanical React Compiler cleanup. Phase E does Wave 6 + 7 validation work on top of the Documenso changes. Phase F mops up Section G dependency migrations + Section D placeholders + Section E hidden tabs. Each phase ships a working, testable cut; commits are atomic per item with a phase-tagged prefix.
|
||
|
||
**Tech Stack:** Same as the parent CRM — Next.js 16 App Router, React 19, TypeScript strict, Drizzle ORM on PostgreSQL, BullMQ + Redis, better-auth, React-Email + nodemailer + imapflow, Documenso v1/v2 client, pdf-lib for AcroForm fill.
|
||
|
||
**Estimate:** ~4 weeks focused work end-to-end. Phase A is the only phase where "ship before deploy" is hard-blocking — everything after is post-launch hygiene.
|
||
|
||
---
|
||
|
||
## Pre-flight (do once)
|
||
|
||
- [ ] **P.1 — Verify session-end state**
|
||
|
||
```bash
|
||
git log --oneline -5
|
||
pnpm exec vitest run
|
||
pnpm exec tsc --noEmit
|
||
pnpm lint
|
||
```
|
||
|
||
Expected: clean tsc, 0 lint errors, 1374/1374 vitest pass (or current baseline). Capture baseline number for "no regressions" gate at end of each phase.
|
||
|
||
- [ ] **P.2 — Verify Wave 2 #12 (admin subject editor) actually outstanding**
|
||
|
||
```bash
|
||
grep -n "overrides?.subject" src/lib/email/templates/*.tsx | wc -l
|
||
```
|
||
|
||
Expected: count ≥ 8 (all templates honour `overrides.subject`). If yes, Wave 2 #12 is **closed in code** by this session's Phase 5 — only the admin UI needs verification. Confirm admin page `/admin/email-templates` exposes a subject input bound to that field; if so, mark #12 ✅ before Phase B starts.
|
||
|
||
- [ ] **P.3 — Refresh backlog tracker**
|
||
|
||
Open `docs/BACKLOG.md` and append a note: "2026-05-18 Part 2 plan live at `docs/superpowers/plans/2026-05-18-audit-cleanup-completion.md`."
|
||
|
||
---
|
||
|
||
## Phase A — Wave 1 stop-ship CRITICALs (~3-5 days)
|
||
|
||
Nine items, half-day each, ship in priority order. **Each item is one commit** with prefix `fix(audit-wave-1): <item>`.
|
||
|
||
### Task A.1 — Real `db:migrate` runner
|
||
|
||
**Why critical:** `0052_audit_critical_fixes.sql` uses `CREATE INDEX CONCURRENTLY` which silently never runs under `db:push`. Six composite indexes missing in prod. Without a real runner this only ever gets worse.
|
||
|
||
**Files:**
|
||
|
||
- Create: `scripts/db-migrate.ts`
|
||
- Modify: `package.json` (replace `db:migrate` script)
|
||
- Test: `tests/integration/scripts/db-migrate.spec.ts`
|
||
|
||
- [ ] **Step 1: Write failing test**
|
||
|
||
```ts
|
||
// tests/integration/scripts/db-migrate.spec.ts
|
||
import { applyPendingMigrations } from '@/scripts/db-migrate-lib';
|
||
import { db } from '@/lib/db';
|
||
import { sql } from 'drizzle-orm';
|
||
import { describe, expect, it, beforeEach } from 'vitest';
|
||
|
||
describe('db-migrate runner', () => {
|
||
beforeEach(async () => {
|
||
await db.execute(sql`DROP TABLE IF EXISTS __drizzle_migrations`);
|
||
});
|
||
|
||
it('applies pending migrations idempotently', async () => {
|
||
const result1 = await applyPendingMigrations({ dir: 'src/lib/db/migrations' });
|
||
expect(result1.applied.length).toBeGreaterThan(0);
|
||
const result2 = await applyPendingMigrations({ dir: 'src/lib/db/migrations' });
|
||
expect(result2.applied.length).toBe(0);
|
||
});
|
||
|
||
it('honours --> statement-breakpoint split and runs CREATE INDEX CONCURRENTLY outside tx', async () => {
|
||
// Verify the runner does NOT wrap CONCURRENTLY statements in a transaction
|
||
// (Postgres rejects CONCURRENTLY inside BEGIN/COMMIT).
|
||
const result = await applyPendingMigrations({ dir: 'src/lib/db/migrations' });
|
||
expect(result.errors).toEqual([]);
|
||
});
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Build the runner**
|
||
|
||
Extract logic into `scripts/db-migrate-lib.ts` (testable) and a thin CLI `scripts/db-migrate.ts` that calls it. Runner contract:
|
||
|
||
1. Read `src/lib/db/migrations/*.sql` in lexical order.
|
||
2. Track applied state in `__drizzle_migrations` table (`id text primary key, applied_at timestamptz`).
|
||
3. Split each file on `--> statement-breakpoint`.
|
||
4. For each statement: if it contains `CONCURRENTLY` → execute outside any tx; otherwise wrap the file's statements in a single tx.
|
||
5. On failure: log statement + abort; do not mark migration applied.
|
||
|
||
- [ ] **Step 3: Rerun vitest, then wire script**
|
||
|
||
```bash
|
||
pnpm exec vitest run tests/integration/scripts/db-migrate.spec.ts
|
||
```
|
||
|
||
Then in `package.json`: `"db:migrate": "tsx scripts/db-migrate.ts"`.
|
||
|
||
- [ ] **Step 4: Backfill missing indexes in dev DB**
|
||
|
||
```bash
|
||
PGPASSWORD=changeme psql -h localhost -p 5434 -U crm -d port_nimara_crm -c "DROP TABLE IF EXISTS __drizzle_migrations;"
|
||
pnpm db:migrate
|
||
```
|
||
|
||
Then verify the 6 indexes from `0052_audit_critical_fixes.sql` exist:
|
||
|
||
```sql
|
||
SELECT indexname FROM pg_indexes WHERE indexname LIKE '%audit_critical%';
|
||
```
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add scripts/db-migrate.ts scripts/db-migrate-lib.ts tests/integration/scripts/db-migrate.spec.ts package.json
|
||
git commit -m "fix(audit-wave-1): real db:migrate runner with CONCURRENTLY support"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.2 — `EMAIL_REDIRECT_TO` production guard
|
||
|
||
**Why critical:** Currently a dev safety net; if accidentally left set in prod, every customer email goes to one dev mailbox.
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/env.ts`
|
||
- Modify: `src/lib/email/index.ts`
|
||
- Test: `tests/unit/lib/env.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
// tests/unit/lib/env.spec.ts (add to existing)
|
||
it('rejects EMAIL_REDIRECT_TO when NODE_ENV=production', () => {
|
||
expect(() =>
|
||
serverEnvSchema.parse({ ...validBase, NODE_ENV: 'production', EMAIL_REDIRECT_TO: 'x@y.com' }),
|
||
).toThrow(/EMAIL_REDIRECT_TO must not be set in production/);
|
||
});
|
||
|
||
it('accepts EMAIL_REDIRECT_TO when NODE_ENV=development', () => {
|
||
expect(() =>
|
||
serverEnvSchema.parse({ ...validBase, NODE_ENV: 'development', EMAIL_REDIRECT_TO: 'x@y.com' }),
|
||
).not.toThrow();
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Update env schema**
|
||
|
||
In `src/lib/env.ts`, add a `superRefine` after the base parse:
|
||
|
||
```ts
|
||
.superRefine((env, ctx) => {
|
||
if (env.NODE_ENV === 'production' && env.EMAIL_REDIRECT_TO) {
|
||
ctx.addIssue({
|
||
code: 'custom',
|
||
path: ['EMAIL_REDIRECT_TO'],
|
||
message: 'EMAIL_REDIRECT_TO must not be set in production',
|
||
});
|
||
}
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Boot warning in mailer**
|
||
|
||
In `src/lib/email/index.ts` at module top:
|
||
|
||
```ts
|
||
if (env.EMAIL_REDIRECT_TO) {
|
||
logger.warn(
|
||
{ emailRedirectTo: env.EMAIL_REDIRECT_TO },
|
||
'EMAIL_REDIRECT_TO is set — every outbound email will be rerouted',
|
||
);
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Tests + commit**
|
||
|
||
```bash
|
||
pnpm exec vitest run tests/unit/lib/env.spec.ts
|
||
git add src/lib/env.ts src/lib/email/index.ts tests/unit/lib/env.spec.ts
|
||
git commit -m "fix(audit-wave-1): reject EMAIL_REDIRECT_TO in production"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.3 — Orphan-blob fix in `handleDocumentCompleted`
|
||
|
||
**Why critical:** Current catch-block leaves blob in storage AND marks `status='completed'` with no `signedFileId`. Storage cost leak + document marked done with no signed PDF retrievable.
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/services/documents.service.ts:1100-1253`
|
||
- Test: `tests/integration/services/documents-completion.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
// tests/integration/services/documents-completion.spec.ts
|
||
it('rolls back storage + DB when files.insert fails mid-completion', async () => {
|
||
// Arrange: stub storage.put to succeed, files.insert to throw
|
||
const putSpy = vi.spyOn(storage, 'put').mockResolvedValueOnce({ key: 'k', sha256: 'x', size: 1 });
|
||
vi.spyOn(db.insert(filesTable), 'values').mockRejectedValueOnce(new Error('boom'));
|
||
|
||
await expect(handleDocumentCompleted({ documentId: doc.id, pdfBytes })).rejects.toThrow();
|
||
|
||
// Assert: blob deleted from storage
|
||
expect(await storage.head('k').catch(() => null)).toBeNull();
|
||
// Assert: documents.status NOT updated to completed
|
||
const after = await db.query.documents.findFirst({ where: eq(documents.id, doc.id) });
|
||
expect(after?.status).not.toBe('completed');
|
||
expect(after?.signedFileId).toBeNull();
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Rewrite as saga**
|
||
|
||
Inside `handleDocumentCompleted`:
|
||
|
||
1. `storage.put(...)` → capture key.
|
||
2. `withTransaction(async (tx) => { insert files, update documents })`.
|
||
3. On tx failure: `await storage.delete(key)` in `finally`/`catch`, then rethrow.
|
||
|
||
Idempotency early-return (`status='completed' && signedFileId`) stays at the top of the function — already handles 5xx retries.
|
||
|
||
- [ ] **Step 3: Verify + commit**
|
||
|
||
```bash
|
||
pnpm exec vitest run tests/integration/services/documents-completion.spec.ts
|
||
git add src/lib/services/documents.service.ts tests/integration/services/documents-completion.spec.ts
|
||
git commit -m "fix(audit-wave-1): handleDocumentCompleted saga compensating delete"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.4 — Escape URLs in email templates
|
||
|
||
**Why critical:** Every template inlines `${data.link}` into `href="…"` without escaping. URL injection risk (template merge tokens could carry a crafted URL).
|
||
|
||
**Note:** `safeUrl()` already exists in `src/lib/email/shell.ts` and is used by most templates. This task sweeps the few that don't use it.
|
||
|
||
**Files (audit each):**
|
||
|
||
- `src/lib/email/templates/portal-auth.tsx`
|
||
- `src/lib/email/templates/inquiry-client-confirmation.tsx`
|
||
- `src/lib/email/templates/inquiry-sales-notification.tsx`
|
||
- `src/lib/email/templates/notification-digest.tsx`
|
||
- `src/lib/email/templates/document-signing.tsx`
|
||
- `src/lib/email/templates/admin-email-change.tsx`
|
||
- `src/lib/email/templates/crm-invite.tsx`
|
||
- `src/lib/email/templates/residential-inquiry.tsx`
|
||
|
||
- [ ] **Step 1: Grep for unsafe usages**
|
||
|
||
```bash
|
||
grep -rn 'href={data\.\|href={`' src/lib/email/templates/
|
||
```
|
||
|
||
For each match: wrap with `safeUrl(...)`. Schema: `safeUrl` already allows only http/https/mailto.
|
||
|
||
- [ ] **Step 2: Add safeUrl coverage test**
|
||
|
||
```ts
|
||
// tests/unit/lib/email/shell.spec.ts (add)
|
||
it.each([
|
||
['javascript:alert(1)', '#'],
|
||
['data:text/html,xss', '#'],
|
||
['ftp://x', '#'],
|
||
['https://ok.com/path', 'https://ok.com/path'],
|
||
['mailto:a@b.c', 'mailto:a@b.c'],
|
||
])('safeUrl(%s) → %s', (input, expected) => {
|
||
expect(safeUrl(input)).toBe(expected);
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git add src/lib/email/templates/ tests/unit/lib/email/shell.spec.ts
|
||
git commit -m "fix(audit-wave-1): route all email template URLs through safeUrl"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.5 — Replace 16 native `window.confirm()` calls
|
||
|
||
**Why critical:** Destructive flows bypass `ConfirmationDialog`/`AlertDialog`. ui-ux-auditor's C1 itemized them. Inconsistent UX + native confirm can't be styled or scripted.
|
||
|
||
**Files (16 sites — grep to enumerate):**
|
||
|
||
```bash
|
||
grep -rn 'window.confirm\|confirm(' src/components/ src/app/ --include='*.tsx' | grep -v node_modules | grep -v 'window\.confirmDocumentMooring'
|
||
```
|
||
|
||
Expected sites include: cancel signing, delete file, archive interest/company/yacht, delete folder, etc.
|
||
|
||
- [ ] **Step 1: Identify shared dialog primitive**
|
||
|
||
`ConfirmationDialog` exists at `src/components/shared/confirmation-dialog.tsx`. Pattern: `useState<boolean>` for open, `<ConfirmationDialog title="…" description="…" confirmLabel="Archive" variant="destructive" onConfirm={…} />`.
|
||
|
||
- [ ] **Step 2: Sweep — 30 min per site × 16**
|
||
|
||
For each `window.confirm(...)` call:
|
||
|
||
1. Add `useState` for dialog open.
|
||
2. Replace the `if (window.confirm(...))` guard with `setConfirmOpen(true)`.
|
||
3. Move action body into `onConfirm` handler.
|
||
4. Render `<ConfirmationDialog open={confirmOpen} onOpenChange={setConfirmOpen} ... />`.
|
||
|
||
- [ ] **Step 3: Add ESLint rule against `window.confirm`**
|
||
|
||
In `eslint.config.mjs`:
|
||
|
||
```js
|
||
{
|
||
rules: {
|
||
'no-restricted-globals': ['error', { name: 'confirm', message: 'Use <ConfirmationDialog> instead' }],
|
||
},
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git add src/components/ src/app/ eslint.config.mjs
|
||
git commit -m "fix(audit-wave-1): replace 16 window.confirm() with ConfirmationDialog"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.6 — GDPR Article-15 export completeness
|
||
|
||
**Why critical:** Regulator-finding-level gap. Current bundle missing: portal_users, email_threads/messages, document_sends, reminders, files, scratchpadNotes, client_merge_log, contact_log, website_submissions, form_submissions.
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/services/gdpr-bundle-builder.ts`
|
||
- Test: `tests/integration/services/gdpr-bundle.spec.ts`
|
||
|
||
- [ ] **Step 1: Enumerate missing relations**
|
||
|
||
For a single test client, seed at least one row in each missing table linking back via `clientId` (or polymorphic FK where applicable).
|
||
|
||
- [ ] **Step 2: Failing test**
|
||
|
||
```ts
|
||
it('includes every PII-bearing relation in the GDPR bundle', async () => {
|
||
const bundle = await buildGdprBundle(testClient.id);
|
||
expect(Object.keys(bundle)).toEqual(
|
||
expect.arrayContaining([
|
||
'client',
|
||
'addresses',
|
||
'contacts',
|
||
'interests',
|
||
'notes',
|
||
'documents',
|
||
'tags',
|
||
// newly required:
|
||
'portalUsers',
|
||
'emailThreads',
|
||
'emailMessages',
|
||
'documentSends',
|
||
'reminders',
|
||
'files',
|
||
'scratchpadNotes',
|
||
'clientMergeLog',
|
||
'contactLog',
|
||
'websiteSubmissions',
|
||
'formSubmissions',
|
||
]),
|
||
);
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Add the 10 missing relation loaders**
|
||
|
||
Each loader is a Drizzle query bounded by `port_id` + `client_id` (or polymorphic resolver where the relation doesn't carry client_id directly — use the symmetric-reach pattern from `listFilesAggregatedByEntity`).
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git add src/lib/services/gdpr-bundle-builder.ts tests/integration/services/gdpr-bundle.spec.ts
|
||
git commit -m "fix(audit-wave-1): GDPR bundle includes 10 missing relations"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.7 — Right-to-be-forgotten true wipe
|
||
|
||
**Why critical:** `client-hard-delete.service.ts` nullifies FKs but leaves verbatim PII in `email_messages.body_html`, `files`, `document_sends.recipient_email`. Doesn't satisfy Article-17.
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/services/client-hard-delete.service.ts`
|
||
- Test: `tests/integration/services/client-hard-delete.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
it('overwrites PII in email_messages.body_html and document_sends', async () => {
|
||
await hardDeleteClient({ clientId, requestedBy: actor });
|
||
const msgs = await db.query.emailMessages.findMany({
|
||
where: eq(emailMessages.clientId, clientId),
|
||
});
|
||
msgs.forEach((m) => {
|
||
expect(m.bodyHtml).toBe('');
|
||
expect(m.bodyText).toBe('');
|
||
expect(m.subject).toBe('[erased]');
|
||
});
|
||
const sends = await db.query.documentSends.findMany({
|
||
where: eq(documentSends.clientId, clientId),
|
||
});
|
||
sends.forEach((s) => expect(s.recipientEmail).toBe('[erased]'));
|
||
});
|
||
|
||
it('deletes file blobs from storage', async () => {
|
||
const file = await db.query.files.findFirst({ where: eq(files.clientId, clientId) });
|
||
await hardDeleteClient({ clientId, requestedBy: actor });
|
||
expect(await storage.head(file!.storageKey).catch(() => null)).toBeNull();
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Add wipe pass**
|
||
|
||
Inside the existing tx:
|
||
|
||
1. `UPDATE email_messages SET body_html = '', body_text = '', subject = '[erased]' WHERE client_id = $1`.
|
||
2. `UPDATE document_sends SET recipient_email = '[erased]' WHERE client_id = $1`.
|
||
3. For each `files` row: delete blob via `storage.delete(key)`, then delete the row. Errors on storage.delete should not block tx — log and continue (blob orphan is acceptable; row deletion is the legally required action).
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git add src/lib/services/client-hard-delete.service.ts tests/integration/services/client-hard-delete.spec.ts
|
||
git commit -m "fix(audit-wave-1): client hard-delete true-wipe PII text + blobs"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.8 — `user_permission_overrides.user_id` FK + `onDelete='set null'`
|
||
|
||
**Why critical:** Orphan rows accumulate when users are deleted.
|
||
|
||
**Files:**
|
||
|
||
- Create: `src/lib/db/migrations/0076_user_permission_overrides_fk.sql`
|
||
|
||
- [ ] **Step 1: Write migration**
|
||
|
||
```sql
|
||
ALTER TABLE user_permission_overrides
|
||
ADD CONSTRAINT user_permission_overrides_user_id_fkey
|
||
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL;
|
||
```
|
||
|
||
- [ ] **Step 2: Apply + verify**
|
||
|
||
```bash
|
||
PGPASSWORD=changeme psql -h localhost -p 5434 -U crm -d port_nimara_crm -f src/lib/db/migrations/0076_user_permission_overrides_fk.sql
|
||
```
|
||
|
||
Then in the Drizzle schema `src/lib/db/schema/permissions.ts` (or wherever `userPermissionOverrides` lives): add `.references(() => users.id, { onDelete: 'set null' })` to the column.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git add src/lib/db/migrations/0076_user_permission_overrides_fk.sql src/lib/db/schema/
|
||
git commit -m "fix(audit-wave-1): user_permission_overrides.user_id FK + set null"
|
||
```
|
||
|
||
---
|
||
|
||
### Task A.9 — Resolve-identifier endpoint replacement
|
||
|
||
**Why critical:** Currently echoes the real canonical email on a successful username hit. Even rate-limited, this is a username-to-email mapping oracle.
|
||
|
||
**Files:**
|
||
|
||
- Delete (or refactor): `src/app/api/auth/resolve-identifier/route.ts`
|
||
- Modify: `src/app/(auth)/login/page.tsx` (form action)
|
||
- Create: `src/app/api/auth/sign-in-proxy/route.ts`
|
||
- Test: `tests/integration/api/auth-sign-in-proxy.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
it('never returns canonical email on identifier+password attempt', async () => {
|
||
// Success path
|
||
const ok = await POST(req({ identifier: 'username1', password: 'correct-pass' }));
|
||
const okBody = await ok.json();
|
||
expect(okBody).not.toHaveProperty('email');
|
||
expect(okBody.data).toMatchObject({ ok: true });
|
||
|
||
// Failure path
|
||
const bad = await POST(req({ identifier: 'username1', password: 'wrong' }));
|
||
const badBody = await bad.json();
|
||
expect(badBody).not.toHaveProperty('email');
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Build proxy**
|
||
|
||
New endpoint accepts `{ identifier, password }`, resolves canonical email server-side, calls `auth.api.signInEmail({ body: { email, password } })`, returns `{ data: { ok: true } }` with session cookie set on success, `{ error: { code: 'INVALID_CREDENTIALS' } }` on failure. Same response for "no such identifier" and "wrong password" (timing-equalize with a dummy bcrypt compare on miss).
|
||
|
||
- [ ] **Step 3: Update login form**
|
||
|
||
`src/app/(auth)/login/page.tsx` posts to the proxy directly with the user-entered identifier; no two-step flow.
|
||
|
||
- [ ] **Step 4: Delete old endpoint**
|
||
|
||
```bash
|
||
git rm src/app/api/auth/resolve-identifier/route.ts
|
||
```
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add src/app/api/auth/sign-in-proxy/ src/app/\(auth\)/login/page.tsx tests/integration/api/auth-sign-in-proxy.spec.ts
|
||
git commit -m "fix(audit-wave-1): sign-in proxy replaces resolve-identifier email leak"
|
||
```
|
||
|
||
---
|
||
|
||
### Phase A acceptance gate
|
||
|
||
- [ ] `pnpm exec vitest run` — all green, ≥ baseline count
|
||
- [ ] `pnpm exec tsc --noEmit` — clean
|
||
- [ ] `pnpm lint` — 0 errors
|
||
- [ ] `pnpm exec playwright test --project=smoke` — green
|
||
- [ ] All 9 commits land
|
||
- [ ] Update `docs/BACKLOG.md` §H Wave 1 — mark all 9 items ✅
|
||
- [ ] Hand off Phase B
|
||
|
||
---
|
||
|
||
## Phase B — Wave 2 HIGH security + observability (~5-7 days)
|
||
|
||
Six items, ~half-day each. Prefix: `fix(audit-wave-2): <item>`.
|
||
|
||
### Task B.1 — `audit_logs.metadata` PII masking + 90-day retention
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/audit.ts` (extend `maskSensitiveFields` to cover `metadata` JSONB)
|
||
- Create: `src/jobs/processors/audit-retention.ts` (90-day cron)
|
||
- Modify: `src/lib/queue/scheduler.ts` (register cron)
|
||
- Test: `tests/unit/lib/audit-mask.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test for masking**
|
||
|
||
```ts
|
||
it('masks email/phone/password/token in metadata JSONB', () => {
|
||
const masked = maskSensitiveFields({
|
||
metadata: { email: 'a@b.c', phone: '+44...', password: 'x', other: 'keep' },
|
||
});
|
||
expect(masked.metadata.email).toBe('[redacted]');
|
||
expect(masked.metadata.phone).toBe('[redacted]');
|
||
expect(masked.metadata.password).toBe('[redacted]');
|
||
expect(masked.metadata.other).toBe('keep');
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Add retention worker**
|
||
|
||
Mirror the `error_events` retention worker. Daily cron deleting rows where `created_at < now() - interval '90 days'`. Register in scheduler.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-2): audit_logs metadata PII mask + 90d retention"
|
||
```
|
||
|
||
### Task B.2 — Webhook → error pipeline
|
||
|
||
**Files (sweep):**
|
||
|
||
- `src/app/api/webhooks/documenso/route.ts`
|
||
- `src/app/api/webhooks/imap-bounce/route.ts` (if any)
|
||
- Any other `/api/webhooks/*` routes
|
||
|
||
- [ ] **Step 1: Locate webhook routes**
|
||
|
||
```bash
|
||
ls src/app/api/webhooks/
|
||
```
|
||
|
||
- [ ] **Step 2: Wrap each handler**
|
||
|
||
```ts
|
||
try {
|
||
await handle(req);
|
||
} catch (err) {
|
||
await captureErrorEvent(err, { source: 'webhook:documenso', requestId });
|
||
throw err;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 3: Test that thrown errors are captured**
|
||
|
||
For each webhook, integration-test that a forced throw lands in `error_events`.
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-2): webhooks emit errors to captureErrorEvent"
|
||
```
|
||
|
||
### Task B.3 — Admin email-template subject editor wiring verification
|
||
|
||
**Status:** Phase 5 of the post-audit master plan rewrote all 8 templates to honour `overrides.subject`. This task confirms the admin UI surfaces it and adds a test.
|
||
|
||
**Files:**
|
||
|
||
- Verify: `src/app/(dashboard)/[portSlug]/admin/email-templates/page.tsx`
|
||
- Verify: `src/lib/services/mailer-config.ts` `getTemplateOverridesForKey()`
|
||
- Test: `tests/integration/services/mailer-config.spec.ts`
|
||
|
||
- [ ] **Step 1: Manually walk admin page** — confirm subject input present + saves.
|
||
- [ ] **Step 2: Integration test that override is applied**
|
||
|
||
```ts
|
||
it('honours subject override from email_templates table', async () => {
|
||
await db
|
||
.update(emailTemplates)
|
||
.set({ subject: 'Custom override' })
|
||
.where(eq(emailTemplates.key, 'portal_activation'));
|
||
const result = await portalAuthEmail(testData, {
|
||
subject: await getTemplateOverridesForKey('portal_activation').subject,
|
||
});
|
||
expect(result.subject).toBe('Custom override');
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Commit (or close as already-done if test passes against current code)**
|
||
|
||
```bash
|
||
git commit -m "test(audit-wave-2): verify admin subject override flows to render"
|
||
```
|
||
|
||
### Task B.4 — Admin signature/footer fields wire-up OR delete UI
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/email/shell.ts` (read `email_signature_html` + `email_footer_html`)
|
||
- Modify: `src/app/(dashboard)/[portSlug]/admin/email/page.tsx` (if deleting)
|
||
|
||
- [ ] **Step 1: Decide — wire or delete**
|
||
|
||
If admin already uses signature/footer in practice (check `system_settings` for non-null rows): wire. If unused: delete the UI and the columns.
|
||
|
||
- [ ] **Step 2 (wire path): Extend shell**
|
||
|
||
`renderShell({...})` accepts optional `signatureHtml` + `footerHtml` from per-port system_settings. Both run through allowlist sanitizer (`isomorphic-dompurify` already in deps).
|
||
|
||
- [ ] **Step 3 (wire path): Plumb in `mailer-config.ts`**
|
||
|
||
Load signature/footer alongside branding in every `send*` site.
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-2): wire admin signature/footer through email shell"
|
||
```
|
||
|
||
### Task B.5 — PII redaction in error pipeline
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/observability/error-events.ts` (or wherever `request_body_excerpt` sanitizer lives)
|
||
- Test: `tests/unit/lib/observability/error-sanitizer.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
it.each([
|
||
['email', { email: 'a@b.c' }, /\[redacted\]/],
|
||
['phone', { phone: '+44 1234 567890' }, /\[redacted\]/],
|
||
['firstName', { firstName: 'Alice' }, /\[redacted\]/],
|
||
['dateOfBirth', { dob: '1990-01-01' }, /\[redacted\]/],
|
||
['address', { addressLine1: '10 Downing St' }, /\[redacted\]/],
|
||
])('redacts %s', (_, input, expected) => {
|
||
expect(JSON.stringify(sanitize(input))).toMatch(expected);
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Extend sanitizer key list**
|
||
|
||
Add: `email`, `phone`, `firstName`, `lastName`, `fullName`, `dateOfBirth`, `dob`, `addressLine1`, `addressLine2`, `city`, `postalCode`, `country`.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-2): error sanitizer redacts PII keys"
|
||
```
|
||
|
||
### Task B.6 — Notification email worker XSS
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/lib/queue/workers/notifications.ts:65-71`
|
||
- Test: `tests/unit/queue/notifications-xss.spec.ts`
|
||
|
||
- [ ] **Step 1: Failing test**
|
||
|
||
```ts
|
||
it('escapes notification description and link in HTML', async () => {
|
||
const html = await buildNotificationHtml({
|
||
description: '<script>alert(1)</script>',
|
||
link: 'javascript:alert(1)',
|
||
});
|
||
expect(html).not.toContain('<script>');
|
||
expect(html).toContain('<script>');
|
||
expect(html).not.toMatch(/href="javascript:/);
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Apply `escapeHtml` + `safeUrl`**
|
||
|
||
Wrap both interpolation sites. `escapeHtml` already exists in `src/lib/email/shell.ts`.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-2): notification email worker escapes user content"
|
||
```
|
||
|
||
### Phase B acceptance gate
|
||
|
||
- [ ] All 6 commits land
|
||
- [ ] No regressions in vitest / tsc / lint / smoke
|
||
- [ ] `docs/BACKLOG.md` §H Wave 2 marked ✅
|
||
|
||
---
|
||
|
||
## Phase C — Documenso phases 2-7 + Section C bundled deferred items (~30-40h)
|
||
|
||
**Sequencing:** Phase 7 first (1h independent pickoff), then Phase 2 (3-4h, foundation for everything else), then Section C Documenso-related items (bundled with Phase 2), then Phase 3 (6-8h custom upload), then Phase 4 (10-14h field placement UI — partially built by Phase 7 template editor this session), then Phase 5 (1-2h verification needing live v2), then Phase 6 polish (each 2-3h, optional).
|
||
|
||
Prefix: `feat(documenso-p<N>): <scope>`.
|
||
|
||
### Task C.1 — Documenso Phase 7 (Project Director RBAC)
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/app/(dashboard)/[portSlug]/admin/documenso/page.tsx`
|
||
- Modify: `src/lib/services/documenso-client.ts` (developer/approver user mapping)
|
||
- Schema fields `documenso_developer_user_id`, `documenso_approver_user_id`, `_label` already exist.
|
||
|
||
- [ ] **Step 1: Add user-picker dropdown to admin page**
|
||
|
||
`<Select>` populated from `/api/v1/users` (current port). On select: store user id + label in `system_settings`. Auto-fill `developer_email` + `developer_name` from picked user.
|
||
|
||
- [ ] **Step 2: Webhook handler matches against linked user's email**
|
||
|
||
In `src/app/api/webhooks/documenso/route.ts`: when handling signing events, if recipient.email matches linked developer/approver user email, also push status update into that user's in-CRM notification.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git commit -m "feat(documenso-p7): Project Director RBAC UI binding"
|
||
```
|
||
|
||
### Task C.2 — Documenso Phase 2 + Section C deferred items (bundled)
|
||
|
||
**Why bundled:** Section C items 1-2 (webhook port_id enforcement, recipient_email dedup) touch the same handler code as Phase 2.
|
||
|
||
**Files:**
|
||
|
||
- Modify: `src/app/api/webhooks/documenso/route.ts` (cascading "your turn" emails, token-based recipient matching, idempotency lock, port_id enforcement)
|
||
- Create: `src/lib/db/migrations/0077_document_events_recipient_email.sql` (add `recipient_email` column + composite unique)
|
||
- Schema: add `recipient_email` to `documentEvents`
|
||
|
||
- [ ] **Step 1: Migration**
|
||
|
||
```sql
|
||
ALTER TABLE document_events ADD COLUMN recipient_email text;
|
||
DROP INDEX IF EXISTS uniq_document_events_body_hash;
|
||
CREATE UNIQUE INDEX uniq_document_events_dedup
|
||
ON document_events (documenso_document_id, recipient_email, event_type)
|
||
WHERE recipient_email IS NOT NULL;
|
||
-- Fallback unique on body_hash for events without recipient context
|
||
CREATE UNIQUE INDEX uniq_document_events_body_hash_fallback
|
||
ON document_events (body_hash)
|
||
WHERE recipient_email IS NULL;
|
||
```
|
||
|
||
- [ ] **Step 2: Enforce port_id**
|
||
|
||
Every `db.query.documents.findFirst({ where: ... })` in the webhook handler gains a `port_id` derivation from the document's signer-set + recipient_email match, asserts the match, refuses to act if mismatched.
|
||
|
||
- [ ] **Step 3: Cascading "your turn" emails**
|
||
|
||
When `RECIPIENT_SIGNED` lands and there's a next signer in sequence (`document_signers.order` ascending), send a "your turn" notification to that signer. Use `document-signing.tsx` template with `nextSignerToken` deep link.
|
||
|
||
- [ ] **Step 4: On-completion PDF distribution**
|
||
|
||
When `DOCUMENT_COMPLETED` lands: pull `documents.completion_cc_emails` (already present from Phase 1) and attach signed PDF to each as a separate send via `sendMail({ to, attachments: [{ filename, content }] })`.
|
||
|
||
- [ ] **Step 5: Token-based recipient matching**
|
||
|
||
Use `document_signers.signing_token` as the lookup key when webhook payload includes a token (v2 `recipient.token` or v1 `signer.signingToken`).
|
||
|
||
- [ ] **Step 6: Idempotency lock**
|
||
|
||
`pg_advisory_xact_lock(hashtext($1))` keyed on `documenso_document_id` for the duration of the handler.
|
||
|
||
- [ ] **Step 7: Tests**
|
||
|
||
```ts
|
||
// tests/integration/webhooks/documenso-cascading.spec.ts
|
||
it('sends your-turn email to next signer in sequence');
|
||
it('attaches signed PDF to completion_cc_emails on COMPLETED');
|
||
it('refuses to act on document with mismatched port_id');
|
||
it('deduplicates per (documensoDocumentId, recipientEmail, eventType)');
|
||
it('serializes concurrent webhook deliveries via advisory lock');
|
||
```
|
||
|
||
- [ ] **Step 8: Commit**
|
||
|
||
```bash
|
||
git commit -m "feat(documenso-p2): webhook cascading + port_id + per-recipient dedup + idempotency"
|
||
```
|
||
|
||
### Task C.3 — Documenso Phase 3 (Custom doc upload-to-Documenso)
|
||
|
||
**Files:**
|
||
|
||
- Create: `src/lib/services/custom-document-upload.service.ts`
|
||
- Create: `src/app/api/v1/interests/[id]/upload-for-signing/route.ts` + sibling `handlers.ts`
|
||
- Create: UI surface — likely a button in `<InterestDocumentsTab>` opening the field-placement UI from Phase 4
|
||
- Test: `tests/integration/services/custom-document-upload.spec.ts`
|
||
|
||
- [ ] **Step 1: Service contract**
|
||
|
||
```ts
|
||
export async function uploadCustomDocumentForSigning(opts: {
|
||
interestId: string;
|
||
uploadedFile: { name: string; bytes: Buffer; mime: string };
|
||
signers: Array<{
|
||
role: 'signer' | 'cc' | 'approver';
|
||
email: string;
|
||
name: string;
|
||
order?: number;
|
||
}>;
|
||
fieldPlacements: Array<{
|
||
recipientEmail: string;
|
||
type: string;
|
||
page: number;
|
||
x: number;
|
||
y: number;
|
||
w: number;
|
||
h: number;
|
||
}>;
|
||
sendImmediately?: boolean;
|
||
}): Promise<{ documentId: string; documentRecordId: string }>;
|
||
```
|
||
|
||
- [ ] **Step 2: Implementation** — v1 path uses `/api/v1/documents` + `/api/v1/documents/{id}/fields` + `/distribute`. v2 path uses `/api/v2/envelope` multipart create + `field/create-many` + `distribute`. Routes through existing `documenso-client.ts` wrappers.
|
||
|
||
- [ ] **Step 3: API route**
|
||
|
||
`withAuth(withPermission('documents.create_for_signing'))`, parseBody schema validates multipart, returns `{ data: { documentRecordId, documensoDocumentId } }`.
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git commit -m "feat(documenso-p3): custom doc upload-for-signing endpoint + service"
|
||
```
|
||
|
||
### Task C.4 — Documenso Phase 4 (Field placement UI)
|
||
|
||
**Note:** Phase 7 template editor (this session) covers click-place / drag / 4-corner resize / multi-page / right-click delete / responsive PDF width / unsaved guard. **Re-use that component.** New piece is the auto-detect anchor scanner via `pdfjs-dist getTextContent`.
|
||
|
||
**Files:**
|
||
|
||
- Modify: existing template editor `src/components/admin/templates/template-editor.tsx` → extract a reusable `<FieldPlacementCanvas>` component
|
||
- Create: `src/lib/pdf/anchor-scanner.ts` (text-position extractor)
|
||
- Create: UI surface where user uploads PDF + previews scanner results
|
||
|
||
- [ ] **Step 1: Extract reusable canvas component**
|
||
|
||
The template-editor.tsx state machine for markers + drag + resize lifts into `<FieldPlacementCanvas pdfUrl onMarkersChange initialMarkers signers />`. Template editor becomes a thin wrapper around it; custom-upload UI uses the same component.
|
||
|
||
- [ ] **Step 2: Anchor scanner**
|
||
|
||
```ts
|
||
export async function scanAnchors(
|
||
pdfBytes: Uint8Array,
|
||
patterns: Array<{ regex: RegExp; fieldHint: string }>,
|
||
): Promise<Array<{ page: number; x: number; y: number; matched: string; fieldHint: string }>>;
|
||
```
|
||
|
||
Uses pdfjs-dist `getTextContent` per page. Default patterns from build-plan Phase 4 (signature, date, full name, initials).
|
||
|
||
- [ ] **Step 3: UI**
|
||
|
||
"Scan for anchors" button on the field-placement screen → shows ghost markers at detected positions → user accepts/rejects → markers go into placement set.
|
||
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git commit -m "feat(documenso-p4): field placement canvas + anchor scanner"
|
||
```
|
||
|
||
### Task C.5 — Documenso Phase 5 + Section C v2 voidDocument verification (bundled)
|
||
|
||
**Note:** Needs a live v2 instance. Defer if unavailable.
|
||
|
||
**Files:**
|
||
|
||
- Verify: `signerMessages` map in website project
|
||
- Verify: `voidDocument` v2 endpoint shape in `documenso-client.ts`
|
||
- Modify: nginx CORS block per integration audit
|
||
|
||
- [ ] **Step 1: With live v2 instance — POST a doc + cancel it**
|
||
- [ ] **Step 2: Confirm `documenso-client.ts:voidDocument` payload shape matches v2**
|
||
- [ ] **Step 3: Update `signerMessages` map across signer-role × documentType combinations**
|
||
- [ ] **Step 4: Apply nginx CORS block**
|
||
|
||
```bash
|
||
git commit -m "feat(documenso-p5): embedded signing URL + v2 void verification"
|
||
```
|
||
|
||
### Task C.6 — Documenso Phase 6 polish (optional, each ~2-3h)
|
||
|
||
Itemize → pick subset based on appetite. Items: auto-send delay, audit-log additions, per-document customisation, document expiration, reminder rate-limit display, failed-webhook recovery UI. Defer all six if appetite is exhausted.
|
||
|
||
### Task C.7 — Section C: public POST routes refactor
|
||
|
||
**Files:**
|
||
|
||
- Create: `src/lib/services/public-interest.service.ts`
|
||
- Modify: `src/app/api/public/{interests,website-inquiries,residential-inquiries}/route.ts`
|
||
|
||
- [ ] **Step 1: Extract `publicInterestService.create(...)` from each public route**
|
||
- [ ] **Step 2: Routes become thin wrappers**
|
||
- [ ] **Step 3: Service unit tests cover dedup, port resolution, branding fan-out**
|
||
- [ ] **Step 4: Commit**
|
||
|
||
```bash
|
||
git commit -m "refactor(public-routes): extract publicInterestService"
|
||
```
|
||
|
||
### Phase C acceptance gate
|
||
|
||
- [ ] Phase 7, 2, 3, 4 ship at minimum (Phase 5 + 6 + 7-refactor optional based on live-v2 availability + appetite)
|
||
- [ ] Section C Documenso-related items closed via Phase 2
|
||
- [ ] Section C public-POST refactor closed via C.7
|
||
|
||
---
|
||
|
||
## Phase D — Wave 3 React Compiler `set-state-in-effect` cleanup (~1 day)
|
||
|
||
40 remaining warnings. Two templates established:
|
||
|
||
- **List/load pattern** → TanStack Query. Template: `src/components/admin/tags/tag-list.tsx`.
|
||
- **Dialog open→reset pattern** → keyed inner DialogBody. Template: `src/components/clients/hard-delete-dialog.tsx`.
|
||
|
||
Prefix: `fix(audit-wave-3): <component>`.
|
||
|
||
- [ ] **Step 1: Enumerate**
|
||
|
||
```bash
|
||
pnpm lint 2>&1 | grep -E 'react-hooks/set-state-in-effect' | sed 's/^.*: //' | sort -u
|
||
```
|
||
|
||
Should yield ~40 file:line tuples.
|
||
|
||
- [ ] **Step 2: Migrate in batches of ~10**
|
||
|
||
For each file:
|
||
|
||
- If `useEffect → fetch → setState` → switch to `useQuery({ queryKey, queryFn })`.
|
||
- If `useEffect` resetting form on prop change → keyed inner component.
|
||
|
||
Run lint after each batch; verify count drops.
|
||
|
||
- [ ] **Step 3: Promote rule to error**
|
||
|
||
In `eslint.config.mjs`:
|
||
|
||
```js
|
||
'react-hooks/set-state-in-effect': 'error',
|
||
```
|
||
|
||
- [ ] **Step 4: Commit (one per batch)**
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-3): migrate batch <N> to TanStack Query"
|
||
```
|
||
|
||
Final commit:
|
||
|
||
```bash
|
||
git commit -m "fix(audit-wave-3): promote set-state-in-effect to error"
|
||
```
|
||
|
||
### Phase D acceptance gate
|
||
|
||
- [ ] `pnpm lint` — 0 `set-state-in-effect` warnings
|
||
- [ ] Rule is `error` in eslint.config.mjs
|
||
- [ ] No vitest / smoke regressions
|
||
|
||
---
|
||
|
||
## Phase E — Wave 6 Documenso depth + Wave 7 reporting/recommender quality (~1 week)
|
||
|
||
Validation work on top of Phase C. Two sub-phases.
|
||
|
||
### Task E.1 — Wave 6 Documenso integration depth audit
|
||
|
||
**Files:** all of `src/lib/services/documenso-client.ts`, `src/app/api/webhooks/documenso/`
|
||
|
||
- [ ] **Step 1: Read `docs/documenso-integration-audit.md` end-to-end**
|
||
- [ ] **Step 2: Live-v2 instance walkthrough — every webhook event type, every API call wrapped by documenso-client**
|
||
- [ ] **Step 3: Verify per-port redirect URL, sequential signing flag, signing-URL emission**
|
||
- [ ] **Step 4: File audit results in `docs/documenso-depth-audit-2026-05-19.md` with findings**
|
||
- [ ] **Step 5: Address any P0/P1 findings — open as commits**
|
||
|
||
### Task E.2 — Wave 6 Email deliverability sanity check
|
||
|
||
- [ ] **Step 1: Verify bounce monitoring against real bounced delivery** (Phase 6 IMAP poller from prior session — needs ops to seed `IMAP_*` env + send a bounce-bait email)
|
||
- [ ] **Step 2: Attachment threshold UX** — confirm 24h signed-URL link path renders sensibly when files exceed `email_attach_threshold_mb`
|
||
|
||
### Task E.3 — Wave 7 reporting math correctness
|
||
|
||
**Files:**
|
||
|
||
- `src/lib/services/reports/revenue.ts`
|
||
- `src/lib/services/reports/pipeline-funnel.ts`
|
||
- `src/lib/services/reports/occupancy.ts`
|
||
|
||
- [ ] **Step 1: Build a deterministic test fixture** — synthetic port with 50 interests across 7 stages over 12 months
|
||
- [ ] **Step 2: Hand-compute expected revenue, funnel rates, occupancy for the fixture**
|
||
- [ ] **Step 3: Snapshot test against current code output; reconcile any divergence**
|
||
|
||
### Task E.4 — Wave 7 berth recommender quality
|
||
|
||
- [ ] **Step 1: Build tier-ladder edge-case fixture** — interests at every boundary (just under/over each tier threshold)
|
||
- [ ] **Step 2: Snapshot expected tier assignments + heat-score order**
|
||
- [ ] **Step 3: Verify against current `berth-recommender.service.ts` output**
|
||
|
||
### Phase E acceptance gate
|
||
|
||
- [ ] Documenso depth audit doc published
|
||
- [ ] Bounce monitoring round-trip green
|
||
- [ ] Reporting + recommender snapshot tests in place + green
|
||
- [ ] Mark Waves 6 + 7 ✅ in BACKLOG
|
||
|
||
---
|
||
|
||
## Phase F — Section G opportunistic + Section D placeholders + Section E hidden tabs (rolling)
|
||
|
||
Lowest priority. Ship as touched.
|
||
|
||
### Task F.1 — Section G `.toLocale*` → `formatDate(...)` rolling sweep
|
||
|
||
93 sites. Migrate when a file is touched for another reason. No standalone PR — flag it in the commit message.
|
||
|
||
### Task F.2 — Section G drizzle-zod for simple validators
|
||
|
||
~28 candidates. Migrate when validator file is touched. Same as F.1.
|
||
|
||
### Task F.3 — Section G `<DataTable virtual />` wiring
|
||
|
||
Three concrete sites: admin/audit-log-list, super-admin port switcher, client export modal. ~15min each.
|
||
|
||
```bash
|
||
git commit -m "perf(audit-section-g): virtualize admin/audit-log-list"
|
||
git commit -m "perf(audit-section-g): virtualize super-admin port switcher"
|
||
git commit -m "perf(audit-section-g): virtualize client export modal"
|
||
```
|
||
|
||
### Task F.4 — Section D scheduler.ts:44 per-user reminder schedule
|
||
|
||
Placeholder — no concrete trigger. Decision: leave as-is, document explicitly in `docs/BACKLOG.md` §D.
|
||
|
||
### Task F.5 — Section D workers/import.ts:13 CSV/Excel import
|
||
|
||
Placeholder — nothing enqueues `import` jobs. Decision: leave as-is, same.
|
||
|
||
### Task F.6 — Section E Interest Contract/Reservation tabs
|
||
|
||
**Gated on:** Phase C (Documenso 2-6) shipping.
|
||
|
||
After Phase C lands, replace the "coming soon" cards with real flow:
|
||
|
||
- Contract tab → custom-doc upload + signing status timeline
|
||
- Reservation tab → berth_reservations CRUD + Documenso send-out
|
||
|
||
```bash
|
||
git commit -m "feat(audit-section-e): interest contract + reservation tabs"
|
||
```
|
||
|
||
### Phase F acceptance gate
|
||
|
||
- [ ] Rolling sweeps documented as "in-progress" in BACKLOG (not "outstanding")
|
||
- [ ] Hidden tabs delivered if Phase C complete
|
||
|
||
---
|
||
|
||
## Final action (end of plan)
|
||
|
||
- [ ] Update `docs/BACKLOG.md` — all addressed sections marked ✅, parked items documented
|
||
- [ ] Update `docs/MASTER-PLAN-2026-05-18.md` — note completion of Part 2
|
||
- [ ] Final session commit: `docs(plan): close audit cleanup Part 2 — sections A, B, C, D, E, F, G, H closed`
|
||
- [ ] Restart dev server + open Playwright for manual verification
|
||
|
||
---
|
||
|
||
## Risk + scope notes
|
||
|
||
- **Phase A is the only hard-blocker.** Everything else is post-launch hygiene.
|
||
- **Phase C Phase 5 needs a live v2 Documenso instance.** Defer if not available; doesn't block 7/2/3/4.
|
||
- **Phase E reporting math** may surface real bugs — budget for follow-up commits.
|
||
- **Phase F.4 + F.5** are explicitly DEFERRED; included so a future engineer sees the explicit decision.
|
||
- If 4 weeks is too long: ship Phase A alone (3-5 days) for production readiness; everything else becomes future-rolling work.
|
||
|
||
---
|
||
|
||
# VERIFICATION SUPPLEMENT (2026-05-18, post-plan)
|
||
|
||
> **This section supersedes the phase scoping above.** A 5-agent Sonnet 1M verification sweep against current `main` found that ~60% of items in the plan are already shipped. Real outstanding scope shrinks from ~4 weeks to ~1.5 weeks of focused work. Read this section first; treat phases A-F above as historical context.
|
||
>
|
||
> Verification methodology: parallel Explore agents with `model: "claude-sonnet-4-6[1m]"` explicit override, narrow scope (4-5 items each), full file:line evidence trail. All findings cross-cite source files at specific lines.
|
||
|
||
## Verified status — 38 items checked
|
||
|
||
### ✅ Already DONE (23 items — no work needed)
|
||
|
||
| Item | Evidence summary |
|
||
| ------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||
| **A.1 — db:migrate runner** | `scripts/db-migrate.ts` (3 modes: apply/status/baseline, CONCURRENTLY-aware, journal-ordered, hash-tracked in `__drizzle_migrations`) |
|
||
| **A.2 — EMAIL_REDIRECT_TO prod guard** | `src/lib/env.ts:115-130` superRefine |
|
||
| **A.3 — Orphan-blob tx in `handleDocumentCompleted`** | `documents.service.ts:1369` idempotency + `:1449` `db.transaction()` + `:1456` SELECT FOR UPDATE re-check + `:1546-1558` compensating `storage.delete(putStoragePath)` |
|
||
| **A.4 — URL escape in templates** | `safeUrl()` consistent across all 7 templates; 0 raw hrefs |
|
||
| **A.5 — `window.confirm()` replacement** | 0 call sites; `use-confirmation.tsx:32` JSDoc references 17 migrated flows |
|
||
| **A.6 — GDPR Article-15 completeness** | `gdpr-bundle-builder.ts:25-244` — all 10 required relations loaded (portalUsers, emailThreads/Messages, documentSends, reminders, files, scratchpadNotes, clientMergeLog, interestContactLog, websiteSubmissions, formSubmissions) |
|
||
| **A.9 — Resolve-identifier replacement** | `sign-in-by-identifier/route.ts` — `.invalid` TLD timing-safe miss response, IP rate-limit with Retry-After, server-side resolution never leaks canonical email |
|
||
| **W2.10 — `audit_logs.metadata` mask + 90d retention** | `audit.ts:274` mask at insert; `maintenance.ts:151-162` retention; `scheduler.ts:72` `'15 6 * * *'` daily cron _(minor camelCase fragment gap — see W2.14 below)_ |
|
||
| **W2.11 — Webhook → error pipeline** | `webhooks/documenso/route.ts:313-323` `captureErrorEvent` wrapped in `withPublicContext` ALS frame |
|
||
| **W2.12 — Subject editor wiring** | 8/8 templates honor `overrides.subject`; 6 use `?.trim()`, 2 use equivalent ternary. Admin UI at `admin/email-templates/page.tsx` |
|
||
| **W2.13 — Signature/footer keys** | Old `email_signature_html`/`email_footer_html` REMOVED; replaced by `branding_email_header_html`/`branding_email_footer_html`, wired through `port-config.ts:100-101`/`:549-550` → `shell.ts:31-32,44-45,69,71` |
|
||
| **W2.15 — Notification email XSS** | `notifications.ts:8` `safeUrl` import + `:12-19` `escapeHtml` + `:82` description escape + `:84` href safeUrl |
|
||
| **A.Phase 3 — Custom-doc upload-for-signing** | `custom-document-upload.service.ts` + `api/v1/interests/[id]/upload-for-signing/route.ts` both shipped |
|
||
| **A.Phase 5 — Embedded signing URL + redirect** | `documenso_redirect_url` end-to-end (registry → port-config:94/408 → documenso-client.ts:314,379 → payload:318); `EmbeddedSigningCard` + test endpoint shipped |
|
||
| **A.Phase 7 — Project Director RBAC** | Registry `user-select` field type at `registry.ts:116-123,162-168`; rendered via `RegistryDrivenForm` in `admin/documenso/page.tsx:244-248`; webhook fires `document_signing_your_turn` in-CRM notification |
|
||
| **A.Risk #4 — v2 payload audit** | `payload.id: number \| string`; `resolveRecipientToken` covers `r.token`/`r.signingToken`/URL fallback (`route.ts:80-85`); doc lookup via `or(documensoId, documensoNumericId)` |
|
||
| **C.1 — Webhook port_id enforcement** | `resolveWebhookDocument` gates with `eq(documents.portId, portId)` (`documents.service.ts:1070-1078`); ambiguous cross-port match refuses mutation; `portScope` forwarded to every event handler |
|
||
| **C.3 — v2 voidDocument shape** | Cleanly branches `v2 → DELETE /api/v2/envelope/{id}` vs `v1 → DELETE /api/v1/documents/{id}` (`documenso-client.ts:1286-1309`); idempotent on 404 |
|
||
| **E.1 — Berth Waiting List + Maintenance tabs** | Removed entirely; comment at `berth-tabs.tsx:449-452` documents the removal |
|
||
| **E.2 — Interest Contract + Reservation tabs** | NOT "coming soon" — both fully wired with active-doc hero card, signers, signing progress, cancel/remind, upload-for-signing, mark-externally-signed |
|
||
| **W3 — set-state-in-effect cleanup** | **0 of 40 remaining.** Entire phase already done. |
|
||
| **W6.bounce — IMAP bounce poller** | `imap-bounce-poller.ts` shipped; scheduled `*/15 * * * *` via `scheduler.ts:59`; no-op when IMAP env unset; state persisted in `system_settings.bounce_poller_state` |
|
||
| **G.4 — Tier-2 deps** | All 12 installed (@sentry, @use-gesture, archiver, embla-carousel, next-intl, pdfjs-dist, react-number-format, react-resizable-panels, react-virtuoso, yet-another-react-lightbox, eslint, type-fest) |
|
||
|
||
### ❌ OUTSTANDING (15 items — real work needed)
|
||
|
||
Ranked by impact + remaining effort:
|
||
|
||
#### Tier 1 — security/data-integrity (ship before next deploy)
|
||
|
||
| Item | Concrete gap | Effort |
|
||
| ------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- |
|
||
| **A.7 — RTBF true wipe** | `client-hard-delete.service.ts` only nullifies FKs. Three gaps: (1) no UPDATE on `email_messages.body_html/body_text/subject`, (2) no UPDATE redacting `document_sends.recipient_email`, (3) `files.clientId` nulled but `storage.delete(key)` never called → orphaned blobs | **4h** |
|
||
| **A.8 — `user_permission_overrides` FK** | Schema declares `onDelete: 'cascade'` (`users.ts:319`), not `'set null'` as audit requires. Migration 0070 skips this table. | **30min** |
|
||
| **W2.14 — PII redaction in error pipeline** | 6 of 13 PII keys NOT caught (firstName, lastName, dateOfBirth, city, postalCode, country) — masker fragments are snake_case but request bodies are camelCase, and the normalizer only swaps hyphens→underscores. Same gap also affects W2.10 (shared masker). A 5xx on `/api/v1/clients` with camelCase payload leaks these into `error_events.request_body_excerpt`. | **2h** |
|
||
|
||
#### Tier 2 — Documenso completion (scope drastically reduced from original plan)
|
||
|
||
| Item | Concrete gap | Effort |
|
||
| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------- |
|
||
| **A.Phase 2 — completion_cc_emails distribution** | Column `documents.completionCcEmails` exists in schema (`documents.ts:103`). `handleDocumentCompleted` never reads it. Only piece of Phase 2 not done. | **2h** |
|
||
| **A.Phase 4 — Auto-detect button wiring** | `document-field-detector.ts` (298 lines, full regex anchor scanner with proximity-based recipient inference) AND `template-editor.tsx` (680 lines, full click-place/drag/resize/multi-page) both exist. Only missing: button in editor that POSTs PDF to a server action calling `detectFields()`, then pre-populates markers. | **3h** |
|
||
| **A.Phase 6 — Polish items** | All four missing: auto-send delay, expiration UI, reminder rate-limit display, failed-webhook recovery UI. Each independent. | **2-3h each, optional** |
|
||
| **C.2 — `documentEvents.recipient_email` column** | Column doesn't exist; only `brochure_sends.recipient_email` exists. Needs migration + composite unique index for per-recipient dedup. | **1h** |
|
||
| **C.4 — `publicInterestService` extraction** | Three public POST routes duplicate inline DB logic. Pure refactor, no active bug. | **2h** |
|
||
|
||
#### Tier 3 — reporting/recommender test coverage (correctness validation)
|
||
|
||
| Item | Concrete gap | Effort |
|
||
| ------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------ |
|
||
| **W7.reports — Math snapshot tests** | Zero coverage in `tests/unit/services/reports/` or equivalent. Only PDF byte smoke tests + E2E clickthrough. Revenue/funnel/occupancy math has never been hand-verified. | **half-day** |
|
||
| **W7.recommender — Tier-ladder snapshots** | Existing `tests/unit/services/berth-recommender.test.ts` has real edge-case coverage with plain `toBe`/`toBeCloseTo` assertions. Not snapshots — limits regression-catching for future weight tuning. | **2h** |
|
||
|
||
#### Tier 4 — opportunistic / rolling
|
||
|
||
| Item | Concrete gap | Effort |
|
||
| ---------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------- |
|
||
| **G.1 — `.toLocale*` migration** | Count grew **93 → 105** sites since audit (more added than migrated). Helper at `format-date.ts` is scaffolded but call sites haven't moved. | **rolling, ~3h opportunistic** |
|
||
| **G.2 — drizzle-zod adoption** | 4 of 40 validator files (10%). Most validators have heavy form transforms that don't 1:1 with table shape — adoption is bounded. | **rolling, 30min/file** |
|
||
| **W6.attach — Attachment threshold UX gating** | Threshold logic + banner exist; **IMAP-gating of the banner** wasn't fully verified at the component level (CLAUDE.md claims it; not seen in send-dialog code). May be working — needs spot-check. | **15min spot-check; up to 1h if broken** |
|
||
| **D.1 — Per-user reminder schedule** | TODO at `scheduler.ts:48` intact. Flat hourly cron only. Defer until customer asks (no concrete trigger). | **deferred** |
|
||
| **D.2 — CSV/Excel import worker** | Worker file exists + registered with BullMQ, but handler is a no-op TODO. No enqueue calls anywhere. Defer (no concrete trigger). | **deferred** |
|
||
|
||
## Revised execution plan — supersedes Phases A-F
|
||
|
||
**Tier 1 (5h total) → ship before next prod deploy.** A.7 + A.8 + W2.14.
|
||
|
||
**Tier 2 Documenso completion (8-10h, mostly optional polish):** A.Phase 2 completion_cc_emails (2h) + A.Phase 4 anchor button wiring (3h) + C.2 recipient_email column (1h) + C.4 service extraction (2h, refactor). A.Phase 6 polish items defer until a customer needs them.
|
||
|
||
**Tier 3 reporting/recommender validation (6h):** Build snapshot fixtures for revenue/funnel/occupancy math + add snapshot variants of recommender tier-ladder tests.
|
||
|
||
**Tier 4 rolling:** G.1 and G.2 migrate opportunistically as files are touched. W6.attach 15-min spot-check. D.1 and D.2 stay deferred.
|
||
|
||
**Total real outstanding work: ~19-25h (~3 focused days).** Down from the original 4-week estimate.
|
||
|
||
## Commit prefixes for this round
|
||
|
||
- `fix(audit-wave-1): A.7 RTBF true wipe`
|
||
- `fix(audit-wave-1): A.8 user_permission_overrides set-null FK`
|
||
- `fix(audit-wave-2): W2.14 PII redaction camelCase coverage`
|
||
- `feat(documenso-p2): completion_cc_emails distribution`
|
||
- `feat(documenso-p4): anchor detector + auto-place button`
|
||
- `feat(audit-c): documentEvents.recipient_email + per-recipient dedup`
|
||
- `refactor(audit-c): extract publicInterestService`
|
||
- `test(audit-w7): reporting math snapshot fixtures`
|
||
- `test(audit-w7): recommender tier-ladder snapshots`
|
||
|
||
## Verification team config (for reference / re-use)
|
||
|
||
Sweep was 5 agents (originally), Sonnet 1M with explicit `model:` param, narrow scope (4-5 items each), pre-quoted evidence where possible:
|
||
|
||
- `wave1-finisher` — A.3/A.6/A.7/A.8
|
||
- `wave2-finisher` — W2.10/W2.11/W2.14/W2.15
|
||
- `documenso-verifier` — A.Phase 2/4/5/6/7 + Risk #4 + C.1/C.3
|
||
- `misc-verifier` — D.1/D.2 + E.1/E.2 + W3 + W6.bounce + W6.attach + W7.reports + W7.recommender
|
||
- `reverify-wave1-quickwins` + `reverify-wave2-and-c` + `reverify-phase3-and-g` — re-did 13 items fresh (per user request to discard inline pre-digest)
|
||
|
||
Lessons captured in `~/.claude/projects/-Users-matt-Repos-new-pn-crm/memory/feedback_subagent_context_bloat.md`.
|