docs(14): add code review report and human verification checkpoint

.planning/phases/14-pax8-ui-surface/14-HUMAN-UAT.md

@@ -0,0 +1,36 @@
+---
+status: partial
+phase: 14-pax8-ui-surface
+source: [14-VERIFICATION.md]
+started: 2026-07-11T19:20:00Z
+updated: 2026-07-11T19:20:00Z
+---
+
+## Current Test
+
+[awaiting human testing]
+
+## Tests
+
+### 1. Re-confirm SC#4 (resolve + persist-through-sync) against the live dev DB, now that CR-01 is fixed
+expected: As an admin, resolve one ambiguous review via a candidate "Link company" button, and one zero-candidate review via the manual-search combobox. In psql, confirm both `pax8_companies.match_method='manual'` (with `autotask_company_id` set) and both `pax8_company_match_review.resolved_at` are set. Trigger a PAX8 sync and confirm neither resolution is reverted or re-flagged.
+result: [pending]
+
+why flagged: 14-06-SUMMARY.md claimed this was already confirmed, but its file timestamp (15:09) precedes both the code review (15:15) and the CR-01 fix commit d56db02 (15:16:38). CR-01 was the bug where the manual-search resolve path 400s on every attempt — so that sub-step of the checkpoint could not have succeeded as described. The live dev DB currently shows 0 resolved matches (38 still open), consistent with the checkpoint not actually having exercised this path successfully. Code is now correct; the live E2E needs to be redone.
+
+### 2. Re-confirm the D-07/D-08 view/resolve permission split at runtime
+expected: As a non-admin authenticated user, `/pax8` and both tabs load, but a resolve attempt (candidate button or manual search) is rejected with a 403 surfaced as an error toast. GET routes succeed for any authenticated session; POST `.../resolve` returns 403 for role='user' and 200 for admin/super-admin.
+result: [pending]
+
+why flagged: Part of the same contested checkpoint above; the underlying role-check logic is code-verified correct but was not independently re-confirmed live after CR-01.
+
+## Summary
+
+total: 2
+passed: 0
+issues: 0
+pending: 2
+skipped: 0
+blocked: 0
+
+## Gaps

.planning/phases/14-pax8-ui-surface/14-VERIFICATION.md

@@ -0,0 +1,123 @@
+---
+phase: 14-pax8-ui-surface
+verified: 2026-07-11T19:20:00Z
+status: human_needed
+score: 4/4 code-level truths verified; 1 live-environment claim contested by evidence
+overrides_applied: 0
+human_verification:
+  - test: "Re-run Plan 14-06 Task 2 Step 6 (resolve one candidate-based review + one manual-search review as admin; trigger a PAX8 sync; confirm both resolutions persist and are not overwritten) against the live dev DB, now that CR-01 is fixed"
+    expected: "Two pax8_companies rows show match_method='manual' with autotask_company_id set; two pax8_company_match_review rows show resolved_at/resolved_by_user_id/resolved_to_company_id set; a subsequent PAX8 sync leaves both untouched (match_method still 'manual', review still resolved)"
+    why_human: "Requires live UI interaction (candidate button + Command/Popover manual search) and triggering a real PAX8 sync against the dev DB — not verifiable by static code inspection alone"
+  - test: "Re-confirm the D-07/D-08 view/resolve permission split against the running app: a non-admin user can view /pax8 and both tabs, but a resolve attempt returns 403"
+    expected: "GET routes succeed for any authenticated user; POST .../resolve returns 403 for a 'user'-role session and 200 for 'admin'/'super-admin'"
+    why_human: "Requires two distinct authenticated browser sessions to exercise both branches at runtime"
+---
+
+# Phase 14: /pax8 UI Surface Verification Report
+
+**Phase Goal:** A manager can open `/pax8` and see PAX8 companies with their subscriptions and a cost breakdown, and an admin can resolve any flagged/ambiguous company match directly from that page — no psql required.
+**Verified:** 2026-07-11T19:20:00Z
+**Status:** human_needed
+**Re-verification:** No — initial verification
+
+## Goal Achievement
+
+### Observable Truths
+
+| # | Truth (Roadmap Success Criterion) | Status | Evidence |
+|---|---|---|---|
+| 1 | `/pax8` lists PAX8 companies together with their current subscriptions | ✓ VERIFIED | `app/api/pax8/companies/route.ts` (requireAuth-gated, whitelisted sort, parameterized search) + `app/pax8/page.tsx` Companies tab wires `DataTable` to it; `npx tsc --noEmit --pretty` clean; live DB has 118 `pax8_companies` / 445 `pax8_subscriptions` rows the route can serve |
+| 2 | Each company shows a cost breakdown by subscription/product, built from synced subscription + order/invoice data | ✓ VERIFIED | `app/api/pax8/companies/[id]/route.ts` correctly windows `DISTINCT ON (subscription_id)` per-subscription latest order-item (not a single global MAX), uses `line_total` (never recomputes `unit_price*qty` when a line total exists), and appends tombstoned-subscription rows. `DetailModal.tsx` renders `PAX8_COMPANY_GROUPS` + an `Array.isArray(data.subscriptions)`-guarded cost table with a summed total, wired via `app/pax8/page.tsx`'s fetch-then-open `handleRowClick` |
+| 3 | Flagged/ambiguous company matches appear in a distinct, clearly-labeled review section, not mixed into the main list | ✓ VERIFIED | `app/pax8/page.tsx` "Needs Review" `TabsContent` renders amber-bordered (`border-amber-200`) `Card`s fed by `GET /api/pax8/company-matches` (separate route, separate tab, distinct from the Companies `DataTable`); count badge on the tab trigger when `reviewTotal > 0`; D-09 zero-candidate empty state present |
+| 4 | From that review section, an admin can pick the correct Autotask company for a flagged match; the resolution persists and is respected by future syncs | ✓ CODE-VERIFIED / ? LIVE-UNCONFIRMED | `resolvePax8CompanyMatch()` (`lib/services/pax8-company-match-resolver.ts`) writes both `pax8_companies.match_method='manual'` and `pax8_company_match_review.resolved_*` atomically inside `postgresClient.transaction()`; `pax8-company-matcher.ts`'s `applyLink`/eligibility query correctly excludes `match_method='manual'` and human-resolved review rows from re-scoring (verified by reading the matcher source, not just the resolver). 5/5 unit tests pass. **However**, the live dev DB currently shows **zero** rows with `match_method='manual'` and **zero** resolved review rows (38 still open, matching the pre-verification baseline) — see Human Verification below for why this contradicts 14-06-SUMMARY.md's claim that this was confirmed end-to-end |
+
+**Score:** 3/4 truths fully code-and-live verified; 1/4 (SC#4) is code-verified but its claimed live confirmation is contested by direct evidence and needs to be redone.
+
+### Required Artifacts
+
+| Artifact | Expected | Status | Details |
+|---|---|---|---|
+| `app/api/pax8/companies/route.ts` | Paginated/sortable/searchable list, requireAuth-gated | ✓ VERIFIED | Whitelisted `SORT_COLUMNS` map, parameterized search/limit/offset, camelCase transform, `requireAuth()` first statement, no `requirePermission` |
+| `app/api/pax8/companies/[id]/route.ts` | Subscriptions + windowed cost breakdown | ✓ VERIFIED | `DISTINCT ON (subscription_id)` windowing present; `line_total` used before falling back to `price*quantity`; UUID validation (loose regex, see WR-04 below); `requireAuth()`-gated |
+| `app/api/pax8/company-matches/route.ts` | Unresolved review queue, requireAuth-gated | ✓ VERIFIED | `WHERE r.resolved_at IS NULL`; bulk `= ANY($1::bigint[])` candidate-name fetch (no N+1); `requireAuth()` only |
+| `lib/services/pax8-company-match-resolver.ts` | Two-table transactional resolver | ✓ VERIFIED | `FOR UPDATE` lock, `not_found`/`already_resolved`/`company_not_found` guards, no candidate-membership check (D-05/D-09), both UPDATEs present |
+| `lib/services/pax8-company-match-resolver.test.ts` | Unit coverage of resolver | ✓ VERIFIED | `npx vitest run lib/services/pax8-company-match-resolver.test.ts` → 5/5 passing |
+| `app/api/pax8/company-matches/[id]/resolve/route.ts` | Admin-gated resolve mutation | ✓ VERIFIED | `requirePermission('admin','access')` first statement; zod-validated body; delegates to resolver inside `postgresClient.transaction()`; result-code → HTTP status mapping correct |
+| `components/admin/DetailModal.tsx` | Additive `kind='pax8_company'` + cost table | ✓ VERIFIED | `PAX8_COMPANY_GROUPS` added; `detectGroups(data, kind)` checks `kind === 'pax8_company'` first, existing `TICKET_GROUPS`/`COMPANY_GROUPS` branches untouched; subscriptions table renders above field groups with `tabular-nums` amounts and a fallback label chain |
+| `app/pax8/page.tsx` | Tab shell, Companies tab, Needs Review tab | ✓ VERIFIED | 502 lines; both tabs fully implemented (no stub remaining); fetch-then-open drill-down; amber review cards; Command/Popover manual search; count badge |
+| `components/navigation/app-navigation.tsx` | Top-level PAX8 nav entry, all-authenticated-user visible | ✓ VERIFIED | `{ title: 'PAX8', href: '/pax8', icon: ShoppingCart, ... }` added to `navigationItems`; NOT added to the `Engagement`/`Admin` super-admin `visibleItems` gate; `MobileNav` consumes the same `visibleItems` array, so mobile gets it too |
+| `app/api/data/companies-list/route.ts` | requireAuth-hardened, bigint-safe | ✓ VERIFIED | `requireAuth()` added (14-05); CR-01 fix (commit `d56db02`) casts `id` to `Number(row.id)` before serializing — confirmed present in current file content |
+
+### Key Link Verification
+
+| From | To | Via | Status | Details |
+|---|---|---|---|---|
+| `app/pax8/page.tsx` | `/api/pax8/companies` | `fetch` in `fetchCompanies` | ✓ WIRED | Confirmed at line 117 |
+| `app/pax8/page.tsx` row click | `/api/pax8/companies/[id]` | fetch-then-open `handleRowClick` | ✓ WIRED | Confirmed at line 223; opens `DetailModal` only after the fetch resolves |
+| `app/pax8/page.tsx` | `DetailModal` | `kind="pax8_company"` | ✓ WIRED | Confirmed at line 496 |
+| `app/pax8/page.tsx` Needs Review tab | `/api/pax8/company-matches` | fetch on tab activation | ✓ WIRED | Confirmed at line 155 (`loadReviews`) |
+| `app/pax8/page.tsx` resolve handler | `/api/pax8/company-matches/[id]/resolve` | `POST { companyId }` | ✓ WIRED | Confirmed at line 196 (`resolve()`) |
+| manual-search combobox | `/api/data/companies-list` | fetch-once + client filter | ✓ WIRED | Confirmed at line 172 (`ensureCompaniesLoaded`) — now returns numeric `id`, matching the resolve route's `z.number()` schema post-CR-01-fix |
+| resolve route | `resolvePax8CompanyMatch` | `postgresClient.transaction(tx => ...)` | ✓ WIRED | Confirmed in `resolve/route.ts` |
+| `pax8-company-matcher.ts` `applyLink`/eligibility query | `pax8_companies.match_method='manual'` guard | `WHERE match_method IS DISTINCT FROM 'manual' AND NOT EXISTS (...resolved_at IS NOT NULL)` | ✓ WIRED (by source read) | Confirmed present in `lib/services/pax8-company-matcher.ts` lines 116-136 and 216-231 — this is the mechanism that is supposed to make SC#4 persistence hold, but has not been exercised against a real resolved row in this environment (see below) |
+
+### Requirements Coverage
+
+| Requirement | Source Plan(s) | Description | Status | Evidence |
+|---|---|---|---|---|
+| PAX8-12 | 14-02, 14-05, 14-06 | Admin views + manually resolves flagged/ambiguous matches | ✓ SATISFIED (code) / ? live-unconfirmed | Resolve route + resolver + Needs Review UI all present and correct; live persist-through-sync demonstration contested (see Human Verification) |
+| PAX8-13 | 14-01, 14-03, 14-04 | `/pax8` lists companies with subscriptions + cost breakdown | ✓ SATISFIED | Companies tab + DetailModal cost table fully wired and code-verified |
+| PAX8-14 | 14-02, 14-05, 14-06 | `/pax8` surfaces flagged matches for manual resolution | ✓ SATISFIED | Needs Review tab distinctly surfaces the queue; resolve wiring present |
+
+No orphaned requirements: REQUIREMENTS.md maps only PAX8-12/13/14 to Phase 14, and all three are declared in plan frontmatter. (Note: REQUIREMENTS.md's own checkbox/table rows for these three still read "Pending" / unchecked — a documentation bookkeeping item, not a functional gap, but worth updating as part of phase close.)
+
+### Anti-Patterns Found
+
+None. Scanned all phase-14-touched files (`app/api/pax8/**`, `app/pax8/page.tsx`, `components/admin/DetailModal.tsx`, `components/navigation/app-navigation.tsx`, `lib/services/pax8-company-match-resolver.ts`, `app/api/data/companies-list/route.ts`) for `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER`/stub patterns — none found. The one `placeholder="Search company…"` match is a legitimate HTML input placeholder attribute, not a debt marker.
+
+Carried forward from `14-REVIEW.md` (already triaged, not newly found, none contradict a must-have truth):
+- **WR-01** (warning): `costTotal` / modal total sum `latestBilledAmount` across subscriptions without grouping by currency — could silently mix currencies for a multi-currency company. Not currently exercised by the dataset (single-currency in practice) but a real correctness gap if it occurs.
+- **WR-02** (warning): `/pax8` page doesn't check `res.ok` before consuming `fetchCompanies`/`handleRowClick` responses — a failed fetch silently renders an empty table or a broken modal instead of an error state.
+- **WR-03** (warning): `PAX8_COMPANY_GROUPS` doesn't surface `matchedCompanyName`/`matchMethod`/`matchConfidence` in the Formatted tab — confirmed still true by reading the current `PAX8_COMPANY_GROUPS` constant (lines 138-160); a user must switch to the Raw tab to see match status.
+- **WR-04** (warning): `UUID_RE = /^[0-9a-f-]{36}$/i` doesn't enforce canonical hyphen positions — malformed-but-same-length ids fall through to a 500 instead of a 400. Confirmed still present in both `companies/[id]/route.ts` and `resolve/route.ts`.
+- **WR-05** (warning): review queue join doesn't filter `pc.is_deleted = false` — a soft-deleted PAX8 company stays in the Needs Review queue indefinitely. Confirmed still present in `company-matches/route.ts`.
+- IN-01 through IN-05 (info): all still present as described in `14-REVIEW.md`, none affect a must-have truth.
+
+None of these warnings/info items block a roadmap success criterion; they are pre-existing findings from the code review, left as-is per the task instructions (lower severity, not necessarily blocking).
+
+### Human Verification Required
+
+#### 1. Re-confirm SC#4 (resolve + persist-through-sync) against the live dev DB, now that CR-01 is fixed
+
+**Test:** As an admin, resolve one ambiguous review via a candidate "Link company" button, and one zero-candidate review via the manual-search combobox. Confirm in psql that both `pax8_companies.match_method='manual'` (with `autotask_company_id` set) and both `pax8_company_match_review.resolved_at` are set. Trigger a PAX8 sync (`POST /api/pax8/sync` or wait for the scheduler) and confirm neither resolution is reverted or re-flagged.
+
+**Expected:** Two companies move from "Needs Review" to resolved; both survive a subsequent sync untouched.
+
+**Why human:** Requires live browser interaction (Command/Popover combobox selection, button clicks) and a real sync run against the dev DB — not verifiable by static code inspection.
+
+**Why this is flagged now, not just routine:** This exact scenario was already claimed as verified in `14-06-SUMMARY.md` ("Human click-through confirmed ... SC#4 (admin resolve persists across a live PAX8 sync)", checkpoint marked "approved"). That claim does not hold up against current evidence:
+
+- The live dev DB right now shows **0** `pax8_companies` rows with `match_method='manual'` and **0** resolved rows in `pax8_company_match_review` (38 open, same count cited as the "known baseline" in `14-VALIDATION.md` before any testing began).
+- `docker logs pulse-app --timestamps` shows the last two PAX8 full-sync runs completed at 14:57:34 and 15:04:39 — **both before** the app container restarted at 15:05:24 to pick up the Phase-14 code. No PAX8 sync (manual or scheduled) has run since the Phase-14 UI went live, through the current time (~4 hours later). The 14-06 checkpoint's step "trigger a PAX8 sync and confirm the two resolutions are NOT overwritten" therefore could not have been exercised against the deployed code as described.
+- More importantly: `14-06-SUMMARY.md`'s file timestamp (15:09) **precedes** `14-REVIEW.md` (15:15) and the CR-01 fix commit `d56db02` (committed 15:16:38). CR-01 is the bug where the manual-search resolve path 400s on every attempt because `companies-list` returned `id` as a JSON string against a strict `z.number()` schema. This means that at the time the human-verify checkpoint was allegedly completed and approved, **the manual-search resolve path was still broken** — so the claimed "resolve...via manual search" sub-step (explicitly listed in the plan's step 6) could not have succeeded as described.
+
+Taken together, this is strong evidence that Task 2 of `14-06-PLAN.md` was not fully and correctly exercised against the live system before being marked "approved," specifically for the manual-search resolve path and the sync-persistence check. The underlying code is now correct (CR-01 fixed, resolver unit-tested, matcher guard read and confirmed) — but the live E2E demonstration needs to be redone and should not be taken on faith from the existing SUMMARY.
+
+#### 2. Re-confirm the D-07/D-08 view/resolve permission split at runtime
+
+**Test:** As a non-admin authenticated user, confirm `/pax8` and both tabs load, but a resolve attempt (candidate button or manual search) is rejected with a 403 surfaced as an error toast.
+
+**Expected:** GET routes succeed for any authenticated session; POST `.../resolve` returns 403 for `role='user'` and 200 for `role='admin'`/`'super-admin'`.
+
+**Why human:** Requires two distinct authenticated browser sessions to exercise both branches; the underlying role-check logic (`lib/permissions.ts`: `user` role has `admin: []`, `admin`/`super-admin` roles have `admin: ["access"]`) is code-verified correct, but this item is part of the same contested checkpoint above and should be re-confirmed alongside it.
+
+### Gaps Summary
+
+No code-level gaps — every artifact, key link, and route matches its plan's must_haves contract, `npx tsc --noEmit --pretty` is clean, and the extracted resolver's 5-case unit test suite passes. The one full-suite failure (`itglue-search.test.ts`, 2/221 tests) is pre-existing and unrelated to Phase 14 (confirmed same failure reproduces on a commit predating Phase 14's first change).
+
+The phase's real open question is process, not code: the "checkpoint:human-verify" gate for SC#4 (`14-06-PLAN.md` Task 2) was marked "approved" in `14-06-SUMMARY.md`, but the timeline of that approval (preceding the CR-01 bug fix) and the current live DB state (zero manual resolutions, no sync run since deployment) directly contradict the specific sub-claims about the manual-search resolve path and sync-persistence having been exercised. This phase cannot be marked fully `passed` until that checkpoint is genuinely re-run against the current (CR-01-fixed) code and its result independently confirmed — which is exactly what the two Human Verification items above ask for.
+
+---
+
+_Verified: 2026-07-11T19:20:00Z_
+_Verifier: Claude (gsd-verifier)_