WFDASH-WEBAPP/docs/qa-review.verify.md

QA Report Verification — docs/qa-review.md

Verifier: verifier agent Date: 2026-07-10 Verdict: PASS (with one cosmetic citation inaccuracy that does not affect the substance)

---

Summary

The producer's QA audit at docs/qa-review.md is honest, evidence-based, and broadly accurate. All three gates (npm run lint, npx tsc --noEmit, npx vitest run) reproduce exactly as claimed. Five spot-checked findings (3 P1 logic bugs, 1 P1 sanitizer risk, 1 P1 dev-script footgun) all hold up when read against the cited files. Coverage is complete across the requested dimensions (build, types, tests, Next 16, PB integration, PB migrations, security, performance, a11y, E2E gaps, dead code). Severity assignments are reasonable — no over-classification spotted. The producer's recommendation (needs fixes, ship-with-known-bugs) matches the evidence on disk.

---

Gate 1 — npm run lint

Method:

cd /Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS && npm run lint 2>&1 | tail -80

Evidence:

/Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS/src/lib/pdf/purchase-order-document.tsx
  122:13  warning  Image elements must have an alt prop ...  jsx-a11y/alt-text
/Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS/src/lib/pdf/quote-document.tsx
  120:13  warning  Image elements must have an alt prop ...  jsx-a11y/alt-text
/Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS/src/lib/server/quotes-service.test.ts
  5:9  warning  'store' is assigned a value but never used  @typescript-eslint/no-unused-vars
/Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS/src/lib/server/quotes-service.ts
  138:12  warning  'err' is defined but never used
  451:9   warning  'receivedAt' is assigned a value but never used
✖ 136 problems (50 errors, 86 warnings)
  2 errors and 29 warnings potentially fixable with the `--fix` option.

Result: PASS — Producer claimed 50 errors / 86 warnings. Reproduced exactly: 50 errors, 86 warnings. Lint categories that appear in the report (react-hooks/set-state-in-effect, @typescript-eslint/no-explicit-any, jsx-a11y/alt-text, etc.) all visible in the raw output.

---

Gate 2 — npx tsc --noEmit

Method:

cd /Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS && npx tsc --noEmit 2>&1 | tail -50

Evidence:

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<InvoiceDetails, "number" | "date">'.

Result: PASS — Producer claimed 1 error at the cited location. Reproduced exactly. Matches P1 #3.

---

Gate 3 — npx vitest run

Method:

cd /Volumes/EXT/DEVELOPMENT/WFDASH_NEXTJS && npx vitest run 2>&1 | tail -120

Note: --reporter=basic is not a valid vitest reporter in this version and errors out; the default reporter was used instead and reproduces the producer's counts.

Evidence (failed tests):

 ❯ src/lib/server/orders-service.test.ts (9 tests | 1 failed) 15ms
     × increments the sequence using the latest matching order_number 5ms
 ❯ src/lib/server/quotes-service.test.ts (59 tests | 4 failed) 35ms
     × starts the month at 001 10ms
     × increments on each call 2ms
     × produces unique numbers under concurrent calls (no race) 3ms
     × creates a new quote, customer, company, items, and attachments 1ms
 ❯ src/app/(app)/collection-pool/page.test.tsx (1 test | 1 failed) 1199ms
     × defaults to operational sort and reorders rows when nearby suppliers is selected

 Test Files  3 failed | 52 passed (55)
      Tests  6 failed | 483 passed (489)

Result: PASS — Producer claimed 6 failed / 483 passed across 3 files. Reproduced exactly: quotes-service.test.ts (4), orders-service.test.ts (1), collection-pool/page.test.tsx (1).

---

Spot-checks — 5 findings verified against source

Spot-check 1: P1 #1 — Quote number formatter (src/lib/server/quotes-service.ts:156-158)

Method: Read the cited lines and the corresponding unit test.

Evidence:

// quotes-service.ts:156-158
function formatQuoteNumber(year: number, month: number, seq: number): string {
  return `Q${year}${String(month).padStart(2, "0")}${String(seq).padStart(3, "0")}`;
}

// quotes-service.test.ts:182 — what the rest of the codebase expects
expect(n).toBe(`Q-${year}-${month}-001`);

// quotes-service.test.ts:367 — n8n v5 contract
expect(result.quote_number).toMatch(/^Q-\d{4}-\d{2}-001$/);

The implemented formatter produces Q202607001; tests and other surfaces (e.g. orders/[id]/page.tsx:1691 placeholder="e.g. Q-2026-04-001") expect Q-2026-07-001. Confirmed bug.

Result: PASS — Bug is real, exactly as described. Fix proposed (add two dashes) is the minimal correct fix.

Spot-check 2: P1 #2 — Order number generator (src/lib/server/orders-service.ts:41-55)

Method: Read the cited lines.

Evidence:

// orders-service.ts:41-55
const now = new Date();
const year = now.getFullYear();
const month = String(now.getMonth() + 1).padStart(2, "0");
const prefix = `ORD-${year}-${month}-`;
const existing = await pb.collection(ORDER_COLLECTION).getList(1, 1, {
  filter: `order_number ~ "${prefix}"`,
  sort: "-order_number",
});
let seq = 1;
if (existing.items.length > 0) {
  const lastNum = parseInt(existing.items[0].order_number.split("-").pop() || "0", 10);
  seq = lastNum + 1;
}

Confirmed: prefix is built from wall-clock now only. If the most recent matching order is from a prior month, the generator will produce ORD-2026-07-008 instead of ORD-2026-06-008. Also confirmed no atomic counter pattern (no order_counters collection mirroring quote_counters).

Unit test (orders-service.test.ts:228):

expect(orders.create.mock.calls[0][0].order_number).toBe("ORD-2026-06-008");

Result: PASS — Bug is real. Both failure modes (month derivation + atomicity) are present.

Spot-check 3: P1 #3 — TypeScript excess-property error (src/app/(app)/collection-confirm/view-model.test.ts:278 vs view-model.ts:210)

Method: Read both cited lines and re-ran npx tsc --noEmit.

Evidence:

// view-model.ts:210
export function buildConfirmNote(invoice: Pick<InvoiceDetails, "date" | "number">): string {
  return `Confirmed for invoicing — ${invoice.number} (${invoice.date})`;
}

// view-model.test.ts:278
expect(
  buildConfirmNote({ date: "2026-07-08", number: "INV-2", setBy: "u", setByName: "n", setAt: "t" }),
).toBe("Confirmed for invoicing — INV-2 (2026-07-08)");

The Pick<InvoiceDetails, "date" | "number"> signature rejects setBy, setByName, setAt. tsc emits error TS2353 as shown in Gate 2 above.

Result: PASS — Bug is real, the proposed fix options (drop fields from test OR widen the type) are both defensible. Report does not force a single choice — correct posture.

Spot-check 4: P1 #4 — Custom HTML sanitizer (src/components/quote-email-card.tsx:166-233)

Method: Read the cited sanitizeHtml function and the call sites.

Evidence:

// quote-email-card.tsx:189
function sanitizeHtml(input: string): string {
  if (typeof window === "undefined" || !input) return "";
  // ... regex-based strip + per-attribute parse ...
}

// quote-email-card.tsx:303 (sanitize wrapper)
return { __html: sanitizeHtml(raw) };

// quote-email-card.tsx:415 (use site)
dangerouslySetInnerHTML={{ __html: bodyHtml }}

Author's own comment at line 171-174: NOT a general-purpose sanitizer (use DOMPurify for that). Confirmed: input is remote email HTML, output is injected via dangerouslySetInnerHTML. The regex parser on line 211 accepts mismatched quote styles (', unquoted values), and the href / javascript: filter on line 220 only catches ASCII javascript: — : (HTML entity-encoded colon) bypasses it. Real XSS risk in a CRM email pipeline.

Result: PASS — Risk is real. Recommended fix (swap to isomorphic-dompurify) is the correct minimal change; 2 vitest payloads (ASCII + entity-encoded) would prove the fix.

Spot-check 5: P1 #5 — scripts/setup-pocketbase.ts:50-63 destructive wipe

Method: Read the cited lines.

Evidence:

// setup-pocketbase.ts:4-5
const ADMIN_EMAIL = "david.els1@outlook.com";
const ADMIN_PASSWORD = "1234567890";

// setup-pocketbase.ts:50-63
const allCollections = ["order_items", "quote_items", "purchase_orders", "orders", "quotes",
  "suppliers", "company_details", "companies", "customers", "stock_items",
  "book_out_records", "logs"];
for (const colName of allCollections) {
  try {
    const existing = await pb.collections.getList(1, 1, { filter: `name = "${colName}"` });
    if (existing.items.length > 0) {
      console.log(`Deleting "${colName}" (${existing.items[0].id})...`);
      const resp = await fetch(`${PB_URL}/api/collections/${existing.items[0].id}`, {
        method: "DELETE",
        ...
      });

Confirmed: hard-coded PB admin credentials committed to source (a public repo could leak, but this is git config user.name = DieselLog, email = dieselog@local on a private repo — still bad hygiene); 12 collections wiped unconditionally on npm run setup against http://127.0.0.1:8090. No --confirm-wipe, no snapshot, no dryRun flag. The npm run setup script is wired in package.json:12 so a typo could wipe local PB data.

Result: PASS — Risk is real. Suggested fixes (env reads + --confirm-wipe + snapshot) are the right minimal shape.

---

Severity sanity check

Spot-audited classifications against the evidence:

Finding	Report grade	My read
Quote formatter produces wrong shape	P1	Correct. Every new quote breaks documented contract; tests fail; downstream n8n indexer reads against the dashed shape.
Order generator picks wrong month + race	P1	Correct. Two distinct bugs (month derivation + atomicity), both in production code.
TS excess-property in collection-confirm test	P1	Correct. CI cannot pass tsc --noEmit — this blocks merges, not just compiles.
Custom regex sanitizer with dangerouslySetInnerHTML	P1	Correct. Author flags it themselves; bypassable on : entity-encoded payloads; email is the canonical XSS ingress for a CRM.
setup-pocketbase.ts unconditional wipe	P1	Borderline-correct. Operator footgun, not a runtime bug. Could argue P2 with hardening, but P1 is defensible because the script is wired to npm run setup and one wrong keystroke drops data.
50 lint errors (27 set-state-in-effect)	P2	Correct. Real React 19 smell; CI gate fails but app still runs.
6 vitest failures	P2	Correct. Tied to P1 fixes.
All pages "use client"	P2	Correct. Performance + first-paint hit; doesn't crash anything.
og-image weights	P2	Correct. Cosmetic-ish, but og-image is referenced by every social preview.
E2E coverage thin	P2	Correct. 5 specs vs 26+ pages.
PB filter string interpolation	P2	Borderline-correct. Pattern survives today; would not survive a refactor. P2 reasonable.
lucide-react@^1.16.0	P2	Correct grade. Report explicitly says "Not a real bug, but flag for a fresh install" — accurate (current npm is 1.24.0; 1.16.0 is in range, lockfile pins it).
Loose PNGs in repo root	P3	Correct. Repo bloat, no runtime impact.
Stray JPG in scripts/	P3	Correct. Same as above.
Loose debug scripts at scripts/dev/	P3	Correct. Pulled into lint scope, not into runtime.
tsconfig excludes scripts/** but ESLint doesn't	P3	Correct. Symptom, not cause.
Modal escape-on-backdrop-click	P3	Correct. UX inconsistency.
ThemeBootstrapScript uses dangerouslySetInnerHTML	P3	Correct. Acceptable because content is a literal in the same file.

No over-classification spotted. No cosmetic typo graded P1. No "code smell" graded P0. No deprecated-but-still-working API graded P0.

---

Coverage check

Required dimension	Covered?	Where
Build	✓ via P2 #4 + "What's Working Well" #8 (next.config.ts:4 standalone output)	OK — did not run npm run build myself but config + lint/tsc/vitest cover the inputs.
Types	✓ — P1 #3 (TSC excess-property)	OK
Tests	✓ — P2 #2 (6 vitest failures, list included)	OK
Next 16 compliance	✓ — "What's Working Well" #1 (async params) + P2 #3 (all "use client")	OK
PB integration	✓ — implicit throughout (counter pattern, filter escape)	OK
PB migrations	✓ — scripts/migrations/ (35 files) referenced in P1 #4 verification flow	Not deeply audited; report acknowledges this via "35 migrations exist"
Security	✓ — P1 #4 (sanitizer) + P1 #5 (hard-coded creds)	OK
Performance	✓ — P2 #3 (no SSR), P2 #4 (og-image weight), P2 #2 (27 set-state-in-effect React 19 cascade)	OK
a11y	✓ — P2 #1 lint count includes jsx-a11y/alt-text (12 false positives)	OK
E2E gaps	✓ — P2 #5 (only 5 specs vs 26+ pages)	OK
Dead code	✓ — P3 #3 + P3 #4 (loose debug scripts + stray JPG)	OK

No missing dimension. Could be deeper on PB migrations (no per-migration audit), but the report's scope is "static QA health check" not "full data-layer audit" — appropriate depth.

---

Missed P0/P1

I deliberately probed for issues the report might have skipped:

• No additional hard-coded secrets in src/. All process.env.PB_ADMIN_* reads are in src/lib/server/pocketbase-admin.ts. Test files use placeholder creds (secret, test-secret-token, admin@example.com) — correct. Only the scripts/setup-pocketbase.ts script has hard-coded admin creds, which the report already flagged (P1 #5).

• No Math.random() for security-relevant IDs. Grep across src/**/*.{ts,tsx} shows no cryptographic-strength requirements being met with Math.random().

• No obviously broken additional tests beyond the 6 cited. The 3-failed / 6-failed count is exact.

• AGENT_INSTRUCTIONS.md is mis-cited. The report (line 39) says:

  AGENT_INSTRUCTIONS.md line 153-156 and docs/SHIPPED.md line 161 both document Q-YYYY-MM-NNN.

  AGENT_INSTRUCTIONS.md does not exist at the project root. The actual files are AGENTS.md, AGENT_BRIEF_AUTOFILL_QUOTES.md, AGENT_BRIEF_AUTOFILL_QUOTES.v1.long.md. The substantive claim (Q-YYYY-MM-NNN is the documented format) is still correct because docs/SHIPPED.md:161 does document it (/api/v1/quotes/next-number returns Q-YYYY-MM-NNN). This is a cosmetic citation error that does not affect the finding's validity.

• Page count "every file under src/app/(app)/**/page.tsx (~24 files)" — actual is 26 client-component pages. Off-by-2 in a rounded estimate; not a defect.

• "R2 — lucide-react@^1.16.0, 1.24.0 is current" — confirmed accurate via npm view lucide-react dist-tags (latest: 1.24.0); package-lock pins 1.16.0; ^1.16.0 resolves to anything <2.0.0. Report's "not a real bug, but flag" framing is correct.

No P0 missed. One minor citation inaccuracy on AGENT_INSTRUCTIONS.md.

---

Adversarial probes (mandatory)

I exercised 3 adversarial probes to try to break the audit:

1. Probe: are the lint counts being reported as one big number or actually broken down by file? Verified that npm run lint produces a per-file breakdown (src/components/order-form.tsx, src/components/quote-pdf-viewer.tsx:47, etc.) matching the report's top-offender list. Not a bulk false-positive count — real files, real rules.

2. Probe: are the vitest failures reproducible at the file+line level? Verified. Each of the 6 failures has a stable file:line citation. The quotes-service.test.ts:182 failure is the same root cause as :192, :207, :367 — the report correctly collapses these to "4 failures, 1 root cause" in P1 #1.

3. Probe: is the dangerouslySetInnerHTML actually wired to sanitizeHtml end-to-end, or is the report citing a dead code path? Verified. sanitizeHtml is defined at line 189, wrapped at line 303 (returns {__html: sanitizeHtml(raw)}), and consumed at line 415 (dangerouslySetInnerHTML={{ __html: bodyHtml }}). Production XSS surface, not a speculative finding.

---

Recommendation

Accept-with-fixes — the audit itself is sound and should be merged as-is. The one cosmetic citation inaccuracy (AGENT_INSTRUCTIONS.md → AGENTS.md / AGENT_BRIEF_AUTOFILL_QUOTES.md) is trivial and does not affect the substance.

The producer's recommended Next Steps (lines 314-322) are sequenced correctly: 1-line formatQuoteNumber fix first, then the view-model.test.ts test/type fix, then the order-month clarification, then DOMPurify swap, then server-side auth gating, then lint-zero, then E2E gap-fill. Agree with the ordering — small fixes before broad perf work.

---

VERDICT: PASS