# WFDASH QA Health Check **Date:** 2026-07-10 **Branch:** `minimax_ui_rev1` **HEAD:** `47b01bd feat: add picking slips, bulk print tracking, supplier map` **Auditor:** Coder (senior QA pass against `Ultimate Debugger and QA Engineer` P0–P3 framework) --- ## 🔍 Debugging Report ### Summary WFDASH is a high-effort, feature-rich Next.js 16 / React 19 / PocketBase CRM with very strong conventions (test scaffold, route guard helpers, sanitizers, async params). But three concrete production-blocking issues surface in **type-check and tests**: the **quote-number formatter** returns `Q202607001` instead of the documented `Q-2026-07-001` shape, the **order-number generator** uses `now` month for its prefix even when the matching sequence comes from a prior month, and the **invoice-details type test** fails TypeScript excess-property checking. Combined with a custom regex HTML sanitizer that the author themselves flags as "not a general-purpose sanitizer", the codebase is in **"ship-with-known-bugs"** territory rather than "in order". Fix the three logic bugs and migrate the sanitizer to DOMPurify before considering the next release. **Verdict:** **Needs fixes** — 3 P1 functional bugs in production code paths + 1 P1 sanitizer risk + manageable secondary issues. --- ### Issues Identified (by severity, descending) #### [P1] — Quote number formatter produces wrong shape - **Location:** `src/lib/server/quotes-service.ts:156-158` - **Severity:** P1 — every new quote, customer-facing, breaks the documented contract. - **Reproduction (unit test, fails today):** ``` npm run test:run FAIL src/lib/server/quotes-service.test.ts > generateQuoteNumberServer > starts the month at 001 expected 'Q202607001' to be 'Q-2026-07-001' ``` Three of the four failure messages are the same root cause (variants of `increments on each call`, `concurrent calls no race`, `createQuoteFromEmail creates a new quote, customer, company, items, and attachments`). - **Root cause:** ```ts function formatQuoteNumber(year, month, seq) { return `Q${year}${String(month).padStart(2, "0")}${String(seq).padStart(3, "0")}`; } ``` Missing the two dashes that every other surface in the codebase assumes. `AGENT_INSTRUCTIONS.md` line 153-156 and `docs/SHIPPED.md` line 161 both document `Q-YYYY-MM-NNN`. The n8n v5 side indexes against this regex (`src/lib/server/quotes-service.test.ts:367`: `/^Q-\d{4}-\d{2}-001$/`). - **Recommended fix:** ```ts return `Q-${year}-${String(month).padStart(2, "0")}-${String(seq).padStart(3, "0")}`; ``` - **Verification:** `npx vitest run src/lib/server/quotes-service.test.ts` shows all four previously-failing tests now green. Run `scripts/smoke/counter.ts` and `counter-concurrent.ts` against dev PB to confirm no other downstream consumer relied on the wrong shape. --- #### [P1] — Order-number generator picks wrong month on rollover - **Location:** `src/lib/server/orders-service.ts:41-55` - **Severity:** P1 — silently resets the sequence at every month boundary; also racy by construction. - **Reproduction (unit test, fails today):** ``` npm run test:run FAIL src/lib/server/orders-service.test.ts:228:57 expected 'ORD-2026-07-008' to be 'ORD-2026-06-008' ``` - **Root cause:** `prefix = ` is built from `new Date()` even when the matched sequence comes from a different month. Two failure modes follow: 1. The unit test pins the contract: when the latest matching order sits in June and we're now in July, the next order should still be `ORD-2026-06-008` — i.e. **the prefix is derived from the matched order, not wall-clock time**. Today the implementation breaks that. 2. Even with a fixed prefix, the generator reads-then-writes without atomic increment — two concurrent `createOrderFromQuote` calls can both observe `ORD-...-007` and both write `ORD-...-008`. (`quotes-service.ts` gets atomicity right via `quote_counters` + `last_seq+` — the orders-side has no equivalent.) - **Recommended fix:** (a) Derive `year`/`month` from the matched order's prefix, fall back to `now` when the table is empty, (b) introduce an `order_counters` collection (mirroring `quote_counters`) for the cross-month rollover, or (c) accept the restart-at-001 behaviour but pin a test that documents it as intentional. The current "use `now` always" path is the worst of the three options. - **Verification:** `npx vitest run src/lib/server/orders-service.test.ts`; then a manual `createOrderFromQuote` after midnight on 2026-06-30/2026-07-01 to confirm what the operator actually sees. --- #### [P1] — TypeScript excess-property error in collection-confirm test - **Location:** `src/app/(app)/collection-confirm/view-model.test.ts:278` (vs `src/app/(app)/collection-confirm/view-model.ts:210`) - **Severity:** P1 — `npx tsc --noEmit` fails; CI cannot pass. - **Reproduction:** ``` src/app/(app)/collection-confirm/view-model.test.ts(278,63): error TS2353: Object literal may only specify known properties, and 'setBy' does not exist in type 'Pick'. ``` - **Root cause:** `buildConfirmNote` is declared `Pick`; the test passes `{ date, number, setBy, setByName, setAt }`. Two fixes are defensible — pick the one that matches intent. - **Recommended fix (pick one):** - Drop the extra fields from the test: ```ts buildConfirmNote({ date: "2026-07-08", number: "INV-2" }) ``` - Or change `buildConfirmNote` to accept the full type if those fields are needed later: ```ts export function buildConfirmNote(invoice: InvoiceDetails): string { … } ``` - **Verification:** `npx tsc --noEmit` shows zero errors. --- #### [P1] — Custom regex-based HTML sanitizer is bypassable - **Location:** `src/components/quote-email-card.tsx:166-233` (`sanitizeHtml`) - **Severity:** P1 — defence-in-depth layer only; author documents this openly but it is invoked with `dangerouslySetInnerHTML` on remote email bodies from arbitrary senders. - **Reproduction (conceptual, not exploited live):** a hostile email body containing `javascript:alert(1)` inside an `` tag, or attribute injection via mismatched quotes (the regex on line 211 accepts `'` and unquoted values, which can smuggle payloads past the per-tag whitelist). - **Root cause:** Regex substitution cannot reliably parse HTML. The author already flags this at line 171-174 ("NOT a general-purpose sanitizer (use DOMPurify for that)"). It survives today because it sees friendly RFQ traffic, but the email pipeline is the canonical XSS ingress for a CRM and the cost of getting it wrong is high. - **Recommended fix:** add `dompurify` + `isomorphic-dompurify`, replace `sanitizeHtml` body with: ```ts import DOMPurify from "isomorphic-dompurify"; return DOMPurify.sanitize(input, { ALLOWED_TAGS: [...ALLOWED_TAGS], ALLOWED_ATTR: [...ALLOWED_ATTRS.a, ...ALLOWED_ATTRS.img, ...ALLOWED_ATTRS.th, ...ALLOWED_ATTRS.td, ...ALLOWED_ATTRS.table], ALLOW_DATA_ATTR: false, }); ``` - **Verification:** add a vitest with two payloads — `javascript:` href, `javascript:alert(1)` href, both should be neutralised. --- #### [P1] — `scripts/setup-pocketbase.ts` is destructive with no backup guard - **Location:** `scripts/setup-pocketbase.ts:50-63` - **Severity:** P1 — operator footgun. Hard-coded admin credentials (line 4-5) `david.els1@outlook.com / 1234567890` and the script unconditionally `DELETE`s 12+ collections against `http://127.0.0.1:8090`. - **Reproduction:** run `npm run setup` against any PB instance that already has data — every listed collection is wiped. - **Root cause:** the comment at line 48-49 acknowledges the destructive intent ("delete all existing (including stale ones from other projects)") but there is no confirmation prompt, no backup step, no `dryRun` flag. - **Recommended fix:** - Move the hardcoded creds to env reads (`PB_ADMIN_EMAIL`, `PB_ADMIN_PASSWORD`) with safe fallback only in `NODE_ENV !== "production"`. - Add a `--confirm-wipe` flag and refuse to run without it. - Add a `takeSnapshot()` call that writes `pb-exports/pb_schema-pre-setup-{ts}.json` first. - **Verification:** run the script against a sandbox PB with non-empty data and confirm: (a) it refuses without the flag, (b) with the flag it produces a snapshot. --- #### [P2] — 50 ESLint errors, dominated by React 19 hook violations - **Location:** see table below - **Severity:** P2 — the build succeeds but CI lint gates will fail, and the new `react-hooks/set-state-in-effect` rule is real: it flags code that triggers cascading renders. - **Reproduction:** `npm run lint`. - **Counts:** | Rule | Count | |---|---| | `react-hooks/set-state-in-effect` | 27 | | `@typescript-eslint/no-explicit-any` | 17 | | `jsx-a11y/alt-text` (false positives on `@react-pdf/renderer` `Image`) | 12 | | `react/display-name` | 1 | | `@typescript-eslint/no-require-imports` | 1 | | `Cannot access variable before it is declared` | 1 | | `react/no-unescaped-entities` | 1 | | **Total** | **50 errors / 86 warnings** | - **Top offender files:** - `src/components/order-form.tsx` (initNewOrder + 4 sub-effects → cascading renders when the form loads) - `src/components/quote-pdf-viewer.tsx:47` (sync reset on close path) - `src/components/supplier-map.tsx:150` (fetch-on-mount in effect) - `src/components/ui/quote-search.tsx:157` (fetch on query change) - `src/app/(app)/collection-map/page.tsx:437,441,504` (4 instances) - `src/app/(app)/quotes/[id]/page.tsx:859,867` (2 instances) - `src/lib/pdf/purchase-order-document.tsx:122` and `quote-document.tsx:120` — false-positive alt warnings on `@react-pdf` `` (PDF renderer, not HTML) - **Recommended fix:** - Where sync state writes happen in cleanup, move them to `useLayoutEffect` (mounted → unmount) or compute from props instead. - Add `// eslint-disable-next-line jsx-a11y/alt-text` on the two PDF `` lines or scope the rule to `.tsx` files matching `**/page.tsx` only. - Replace `any` annotations with `unknown` + narrow — or `as` for genuinely-opaque third-party shapes. - **Verification:** `npm run lint` shows 0 errors, 0 warnings (or warnings only). --- #### [P2] — 6 Vitest failures in production code paths - **Location:** see counts - **Severity:** P2 — same root causes as P1 #1, #2 (quote/order numbering) and P1 #3 (collection-confirm test). Plus one UI-only test. - **Reproduction:** `npx vitest run`. ``` Test Files 3 failed | 52 passed (55) Tests 6 failed | 483 passed (489) ``` - **Failures:** | File | Test | Root cause | |---|---|---| | `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > starts the month at 001` | See P1 #1 | | `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > increments on each call` | See P1 #1 | | `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > produces unique numbers under concurrent calls (no race)` | See P1 #1 (and possibly also a race) | | `src/lib/server/quotes-service.test.ts` | `createQuoteFromEmail > creates a new quote, customer, company, items, and attachments` | See P1 #1 | | `src/lib/server/orders-service.test.ts` | `createOrderFromQuote > increments the sequence using the latest matching order_number` | See P1 #2 | | `src/app/(app)/collection-pool/page.test.tsx` | `CollectionPoolPage sort control > defaults to operational sort and reorders rows when nearby suppliers is selected` | UI: rendered DOM shows rows `[Alpha, Charlie]` not `[Alpha, Bravo, Charlie]` — likely the "nearby" sort colocates Bravo/Charlie by shared coords in the test fixture | - **Recommended fix:** the first 5 fall out of the P1 fixes. For the sort test, inspect `src/app/(app)/collection-pool/page.test.tsx` fixture and the `nearby` comparator at `src/app/(app)/collection-pool/page.tsx` (search for `nearby`). - **Verification:** `npx vitest run` reports 0 failures. --- #### [P2] — All pages are `"use client"`; nothing is server-rendered - **Location:** every file under `src/app/(app)/**/page.tsx` (~24 files), `src/app/layout.tsx` wraps `AuthProvider` (client), and `src/app/(app)/layout.tsx:1` is `"use client"`. - **Severity:** P2 — performance + SEO + first-paint hit. Dashboard, list pages, PDF view all hydrate fresh in the browser. - **Recommended fix:** the (app)/layout auth check should move to a server component that reads the PB auth cookie via `await cookies()` (per Next 16 docs, `01-app/.../cookies.md`), letting list pages render server-side. The `` and provider wrappers stay client-side. - **Verification:** Lighthouse mobile run on `/dashboard` and `/quotes` shows improved LCP and a smaller initial JS payload. --- #### [P2] — `og-image.png` and `og-square.png` are too large for social cards - **Location:** `public/og-image.png` (732K), `public/og-square.png` (1.1M), `public/twitter-image.png` (772K). - **Severity:** P2 — page weight + slow social-share unfurls. `og-image` is referenced on every `metadata.openGraph.images` so it's effectively eagerly fetched by social previews. - **Recommended fix:** re-export at ≤ 200K each as JPEG/WebP using the existing `letterhead` assets. Next/Image `priority` is not used here; add a `public/og-square.{webp,jpg}` and reference in `layout.tsx:111-119,123`. - **Verification:** `du -sh public/og-*` ≤ 200K and a manual Slack/Twitter card preview. --- #### [P2] — E2E coverage is narrow vs page surface - **Location:** `e2e/` (only 5 specs: `_helpers`, `agent-api`, `auth`, `health`, `management`). - **Severity:** P2 — 22+ pages have no E2E coverage. Recent feature work on `invoicing`, `collection-confirm`, `collection-map` (date filter + unassign), `collection-pool` (nearby sort), `sprint` (categories + claims), `purchase-orders` (PDF tokens + additional attachments), and `companies-coordinates` is entirely uncovered at the E2E layer. - **Recommended fix:** add at minimum specs for `invoicing`, `purchase-orders/{id}/pdf`, `collection-pool` (sort + Mark Ready dialog), `sprint/items/{id}/claim`. The unit coverage is strong (`483/489`) but it tests PB services and isolated components, not composed flows. - **Verification:** `npx playwright test --list` shows the new specs and they pass locally. --- #### [P2] — Aggressive PB filter string interpolation - **Location:** `src/lib/pb-helpers.ts` (multiple sites: lines 437, 577, 947, 1066, 1436, 1439). - **Severity:** P2 (the dominant flows use it correctly, but the surface is large). - **Reproduction (conceptual):** in `await pb.collection("orders").getList(1, 50, { filter: `order_number ~ "${search}"` })`, a `search` containing `"` is escaped (`\"`) and newlines/backslashes are not. The resulting filter is well-formed today because every call site uses IDs or short alphanumeric tokens, but the pattern does not survive a refactor. - **Root cause:** `pb-helpers.ts:1436-1439` shows two calls in a row: ```ts clauses.push(`category = "${filters.category.replace(/"/g, '\\"')}"`); clauses.push(`brand = "${filters.brand.replace(/"/g, '\\"')}"`); ``` NB: PB string literals also treat `\` specially; the escape omits that. The PB native SDK offers `pb.filter(expr, params)`-style helpers in 0.21+ but the project uses raw strings. - **Recommended fix:** introduce one internal `pbFilter(field, value, op)` helper in `src/lib/pb-helpers.ts` that handles quote/backslash escaping once and test it. - **Verification:** add a unit test passing `'; --` and `\\` as inputs; the resulting filter string either succeeds (escaped) or throws the expected PB error envelope. --- #### [P2] — `lucide-react` dependency version looks unusual - **Location:** `package.json:58` declares `"lucide-react": "^1.16.0"`. - **Severity:** P2 — verified on npm that lucide-react actually publishes 1.x via the maintainers (1.24.0 is current); package-lock confirms 1.16.0 is the installed version. **Not a real bug**, but flag for a fresh install — if the upstream version diverges from the lockfile's lockSpec pattern, imports break. - **Recommended fix:** in CI, `npm ci` (strict lockfile) so an upstream version drift never surprises a build. Add `engines.npm` if not present. - **Verification:** `npm ci` on a fresh checkout succeeds. --- #### [P2] — Destructive "wipe" in `scripts/setup-pocketbase.ts` (See also P1 #6 — that's the same script; the duplication is intentional here to call out the operator UX angle separately for triaging.) Already covered. --- #### [P3] — Loose PNG/screenshot files in the repo root - **Location:** repo root: `Screenshot 2026-07-09 at 01.12.32.png` (80K), `bulk_modal.png` (177K), `invoicing-*.png` × 6 (~800K total), `page-overview.png` (101K), `single_modal.png` (183K). - **Severity:** P3 — repo bloat, accidentally-committed investigation screenshots. Not referenced by code. - **Recommended fix:** move to `docs/_screenshots/` or `.gitignore` patterns like `Screenshot *.png`, `bulk_modal.png`, `single_modal.png`, `invoicing*.png`, `page-overview.png`. - **Verification:** `git ls-files | grep -E '\.(png|jpg)$' | grep -v public/` returns nothing. --- #### [P3] — Stray large JPG inside `scripts/` - **Location:** `scripts/mrbxnim4-telegram-cloud-photo-size-4-6016860144018328845-y.jpg` (52K) - **Severity:** P3 — looks like a Telegram-saved reference photo that escaped review. - **Recommended fix:** move to `docs/_references/` or delete. --- #### [P3] — Loose ESM debug scripts at `scripts/dev/` - **Location:** `scripts/dev/debug-*.{ts,tsx}` (12 files). - **Severity:** P3 — pulled into ESLint scope (driving the "50 errors" count up). Code in them is incidental. - **Recommended fix:** move to `scripts/dev/_archive/` or add `scripts/dev/**` to `eslint.config.mjs` `globalIgnores`. Either is fine — naming should signal "operator-only, not shipped". - **Verification:** `npm run lint` count drops; nothing in production import-path references them. --- #### [P3] — `tsconfig.json` excludes `scripts/**` but ESLint scans them - **Location:** `tsconfig.json:42` vs `eslint.config.mjs:9-15`. - **Severity:** P3 — symptom, not cause. Many of the 50 lint errors live in scripts that aren't type-checked. Either fix the scripts or exempt them. - **Recommended fix:** add `scripts/**` to the ESLint `globalIgnores`. --- #### [P3] — Modal escape-on-backdrop-click behaviour - **Location:** Base UI `Dialog` (in `src/components/ui/dialog.tsx`) doesn't ship escape-on-backdrop-click out of the box on this version's setting; custom escape handler in `quote-pdf-viewer.tsx:90-97` only. - **Severity:** P3 — minor UX inconsistency. Verify by clicking outside the dialog or pressing `Esc`. - **Recommended fix:** confirm with a manual smoke pass and standardise on the Base UI default. --- #### [P3] — `ThemeBootstrapScript` uses `dangerouslySetInnerHTML` - **Location:** `src/components/theme-bootstrap.tsx` (script with `dangerouslySetInnerHTML={{__html: code}}`) - **Severity:** P3 — acceptable because `code` is a literal authored string in the same file (no remote content), but flag for future readers. --- ### What's Working Well ✅ - **Test scaffolding is healthy.** 483/489 unit tests pass; failures are concentrated in 3 files and tied to the P1 findings (not flaky code). - **Next.js 16 async params** are used consistently in every `[id]/route.ts` and `[id]/page.tsx`. - **shadcn/ui base-nova + Base UI primitives** are used consistently across all 14 base components (`button`, `dialog`, `dropdown-menu`, `input`, `popover`, `scroll-area`, `select`, `separator`, `sheet`, `switch`, `tabs`, `avatar`, `badge`, `checkbox`). Zero Radix in `src/components/ui/`. - **Heavy API routes declare `runtime = "nodejs"` + `dynamic = "force-dynamic"` correctly** (`/api/v1/quotes/[id]/pdf`, `/purchase-orders/[id]/dispatch`, `/purchase-orders/[id]/send-email`, etc.). - **PB filter fallback pattern** for the legacy vs template schema is in place (`pb-helpers.ts:228-260`, "Try the new filter first; fall back to legacy"). - **File proxy at `/api/v1/files/[collection]/[recordId]/[filename]`** has an explicit whitelist, inline-vs-attachment disposition, and cache headers (`no-cache`). - **Theme bootstrap script** prevents FOUC via `suppressHydrationWarning` + pre-paint script. - **`.gitignore`** correctly excludes `.env*`, `.mcp.json`, `test-results/`, `.next/`, `tsbuildinfo`. - **No `TODO`/`FIXME`/`XXX`/`HACK` markers** anywhere under `src/` or `scripts/`. - **PWA scaffold** present (`/manifest.json`, `service-worker-registration`, `/sw.js`). - **Standalone Next.js output** in `next.config.ts:4` — matches the Dockerfile multi-stage build. - **`allowedDevOrigins` includes `192.168.1.122`, `192.168.1.128`** — operator explicitly accommodated LAN phone testing; correct. --- ### Recommendations 1. **Adopt a `react-dom/test-utils` lint baseline for `react-hooks/set-state-in-effect`.** The 27 `set-state-in-effect` violations are a real React 19 smell. Convert the cleanest 3-5 first (`order-form.tsx`, `quote-search.tsx`, `supplier-map.tsx`'s `fetchSuppliers()`) to a `useEffect(() => { let cancelled = false; ... }, [])` pattern with derived state via `useState` initialiser; lint baseline should land at zero (or scoped-down per directory). 2. **Move auth gating server-side.** Replace the `(app)/layout.tsx` client redirect (`src/app/(app)/layout.tsx:12-16`) with a server component that reads the PB auth cookie via `await cookies()`. This unlocks server-side data fetching for the 24 `use client` pages — biggest single perf win available. 3. **Atomic counter for orders** (mirroring `quote_counters`). Even if the month-rollover logic is intentionally fixed to use `now`-month only, two simultaneous `createOrderFromQuote` calls still race. Add `scripts/migrations/2026-07-10-create-order-counters.ts`. 4. **DOMPurify swap-in for the email sanitizer.** Use `isomorphic-dompurify`; same ALLOWED_TAGS/ALLOWED_ATTRS. Add 2 vitest cases covering `javascript:` and `:` payloads. 5. **Convert tests under `src/lib/server/quotes-service.test.ts` and `orders-service.test.ts` to a CI gate.** Add a `lint:fix` script that auto-applies the 29 fixable warnings and gates the build. 6. **Add E2E coverage for invoicing and PO PDF dispatch flows.** The new PDF-token + attachment work has unit coverage but no composed-flow coverage; one Playwright spec per flow closes the gap. --- ### Next Steps > Tackle in this order: > > 1. **Fix `formatQuoteNumber`** — 1 line, unblocks 4 tests + downstream n8n. > 2. **Fix `buildConfirmNote` test (or type)** — 1 line, unblocks `tsc --noEmit`. > 3. **Fix `createOrderFromQuote` month derivation** — clarify intent with the team (extend latest vs. reset-at-month-boundary), pin a test, ship. > 4. **Move auth gating server-side** — biggest perf win, broad but mechanical. > 5. **DOMPurify** — small, immediate XSS risk reduction. > 6. **Lint zero** — schedule a 1-2 hour pass to land `react-hooks/set-state-in-effect` + `no-explicit-any` cleanups. > 7. **E2E gap-fill** for invoicing + PO dispatch. > > Want me to: > - Generate the fixes for the top 3 P1s? > - Open follow-up issues for the lint + e2e gaps? > - Run a security audit pass (XSS, CSRF, rate limiting)?