docs(22): research phase domain for approval UI (LiveLink)

.planning/phases/22-approval-ui-livelink-addressable-campaign-review-and-approve/22-RESEARCH.md

@@ -0,0 +1,520 @@
+# Phase 22: Approval UI (LiveLink) - Research
+
+**Researched:** 2026-07-16
+**Domain:** Next.js App Router page + API route extension over an existing Postgres-backed phishing-triage service layer (no new external integrations)
+**Confidence:** HIGH
+
+## Summary
+
+This phase is almost entirely internal-codebase archaeology, not new-technology research: every API this page calls, every table it reads, and every permission model it must mirror already exists and was read in full during this research pass. The three questions CONTEXT.md deferred to research all have clear, evidence-backed answers:
+
+1. **Extend `GET /api/phishing/campaigns/[id]` in place.** It has exactly one consumer (this route's own tests/callers — none found elsewhere in the codebase besides the route itself), so extending its response shape is safe. No new endpoint needed.
+2. **Ticket→campaign resolution should be a thin server-side helper reused by a new route**, not a new standalone lookup service — `reports.ticket_id` is a direct FK to `tickets.id` (the same AT numeric ticket ID the existing `/api/phishing/tickets/{ticket_id}/analyze` route already keys on), so the join is a single `SELECT campaign_id FROM reports WHERE ticket_id = $1`.
+3. **Blast-radius MUST be fetched fresh (`getBlastRadius()`) on every page load — there is no viable persisted alternative.** This is the most important finding of this research: `classifications.reasons` never stores the structured `BlastRadiusResult` object (matched/delivered/held/rejected/clicked/perRecipient) — the classifier (`campaign-classifier.ts`) only writes short human-readable strings into `reasons`, and only when blast-radius is *unavailable*. When blast radius IS available, nothing about it is persisted anywhere. The "read from persisted `reasons`" option floated as a possibility in CONTEXT.md's Claude's-Discretion section is **not actually implementable** as stated — REVIEW-03's full-detail requirement (counts + per-recipient table) can only be satisfied by a fresh `getBlastRadius()` call, matching Phase 21's `triage-note-service.ts` precedent (which itself always calls it fresh, though with a bug worth flagging — see Pitfall 3).
+
+A second load-bearing gap found during this research: **`remediation_actions` has no `completed_at` column** (confirmed against `migrations/097_phishing_triage_schema.sql` and all migrations through `099`). The UI-SPEC's "Already remediated on {completed_at date}" copy has no direct column to read. The correct, migration-free fix is to derive that timestamp from the matching `audit_events` row (`event_type = 'remediation_completed'`, `payload->>'actionId' = remediation_actions.id`) — both because CONTEXT.md's phase boundary explicitly excludes changing Phase 20's schema/services, and because the audit trail already carries exactly this timestamp.
+
+**Primary recommendation:** Extend the existing campaign-detail route in place with four additive fields (`remediationActions`, `auditEvents`, full `messages`/`classifications` shapes, `blastRadius`), add one new thin ticket→campaign resolver route, derive `completed_at` from `audit_events` rather than adding a migration, and build the client-side permission-gating pattern fresh (no existing `hasPermission()`-client-side precedent exists in this codebase today — the closest analogs use direct `role === 'admin'` string checks).
+
+## Architectural Responsibility Map
+
+| Capability | Primary Tier | Secondary Tier | Rationale |
+|------------|-------------|----------------|-----------|
+| Ticket→campaign resolution | API / Backend | — | New route, DB-only lookup, no external calls — belongs in `app/api/phishing/tickets/[ticket_id]/campaign/route.ts` per CLAUDE.md's "no `'use server'`, everything is API routes" rule |
+| Campaign detail (evidence/timeline/classification) | API / Backend | Database | Extends existing `GET /api/phishing/campaigns/[id]` — all data already in Postgres, no new external calls |
+| Blast-radius freshness | API / Backend | External (Mimecast, via existing client) | Fetched server-side inside the extended detail route (never client-side — the Mimecast client/creds are server-only) |
+| Approve/remediate/mark-false-positive actions | API / Backend | Database | Reuses existing Phase 20 routes verbatim — this phase adds zero new write logic |
+| Permission gating (button disabled/hidden) | Browser / Client | API / Backend (source of truth) | Client-side check is UX-only and must mirror server (`requirePermission`) exactly — server remains the enforcement boundary |
+| Campaigns list page | Browser / Client | API / Backend | Reuses existing `GET /api/phishing/campaigns` (Phase 18) — only additive field is `firstReportTicketId` |
+| Nav entry | Browser / Client | — | Static array edit in `components/navigation/app-navigation.tsx`, no data dependency |
+
+## Standard Stack
+
+No new libraries. This phase composes existing Pulse primitives exclusively.
+
+### Core (existing, reused)
+| Library | Version | Purpose | Why Standard |
+|---------|---------|---------|--------------|
+| Next.js App Router | 16.1.1 | Page routes + API routes | Existing project convention |
+| shadcn/ui (Radix) | existing | Card, Tabs, Accordion, Alert, AlertDialog, Checkbox, Select, Table, Button, Badge | Already initialized (`components.json`) |
+| `pg` via `postgresClient` singleton | 8.11.0 | All new queries | No ORM per CLAUDE.md |
+| `sonner` | 2.0.7 | Toast feedback for approve/remediate/mark-false-positive | Existing convention |
+| `lucide-react` | 0.562.0 | Icons (`ShieldAlert`, `FileText`, `Sparkles`, `CheckCircle2`, `ShieldCheck`, `XCircle`, `Copy`, `SearchX`) | Existing convention |
+
+### Supporting — one new shadcn primitive
+| Library | Version | Purpose | When to Use |
+|---------|---------|---------|-------------|
+| `components/ui/tooltip.tsx` | shadcn official registry | Disabled-button explanations (D-05/D-06) | Run `npx shadcn add tooltip` — confirmed absent from `components/ui/` via grep; every other primitive the UI-SPEC needs (checkbox, tabs, accordion, alert, alert-dialog, card, select, table, button, badge, separator, collapsible) already exists |
+
+### Alternatives Considered
+| Instead of | Could Use | Tradeoff |
+|------------|-----------|----------|
+| Extending `GET /api/phishing/campaigns/[id]` in place | A new `GET /api/phishing/campaigns/[id]/review` endpoint | Rejected — no second consumer to protect, and it would duplicate the reports/messages/indicators/classifications bulk-fetch queries already in the existing route. Extending in place is strictly less code. |
+| Deriving `completed_at` from `audit_events` | Adding a `completed_at` column via new migration + service edit | Rejected for this phase — CONTEXT.md's phase boundary explicitly excludes changing Phase 20's remediation-service.ts/schema. The audit-derived timestamp is available today with zero schema risk. |
+| Server-merging the timeline (reports + classifications + audit events) in the extended route | Client-side merge of three separately-fetched arrays | Recommend server-merge (single sorted array in the response) — CONTEXT.md leaves this to planner's discretion; server-merge avoids duplicating sort/type-tagging logic in the client and keeps the client component a pure renderer. Both are viable; server-merge is the tidier one given `audit_events.campaign_id` is a plain UUID column (not FK-cascaded) alongside `reports`/`classifications` already queried in the same route. |
+
+**Installation:**
+```bash
+npx shadcn add tooltip
+```
+
+**Version verification:** All other dependencies are already installed and pinned in `package.json` (Next.js 16.1.1, React 19.2.3, `lucide-react` 0.562.0, `sonner` 2.0.7) — no version drift risk since nothing new is being added except the one shadcn primitive, which is generated source code (not an npm dependency) and pulled from the official shadcn registry configured in the project's `components.json` (`"registries": {}` — official only, per UI-SPEC's Registry Safety table).
+
+## Package Legitimacy Audit
+
+No new npm packages are introduced by this phase — `npx shadcn add tooltip` generates a local Radix-based component file from the already-configured official shadcn registry (same trust boundary as every other `components/ui/*.tsx` file already in the repo). The Package Legitimacy Gate does not apply; there is nothing to run `slopcheck`/`npm view` against.
+
+**Packages removed due to slopcheck [SLOP] verdict:** none — no packages evaluated (none introduced).
+**Packages flagged as suspicious [SUS]:** none.
+
+<phase_requirements>
+## Phase Requirements
+
+| ID | Description | Research Support |
+|----|-------------|------------------|
+| REVIEW-01 | Stable ticket-ID-addressable route resolving ticket→campaign, Better Auth session only | Confirmed `reports.ticket_id` is a direct BIGINT FK to `tickets.id` (the AT numeric ticket ID) — same param semantics as the existing `/api/phishing/tickets/{ticket_id}/analyze` route. `/phishing` is absent from `middleware.ts`'s `publicRoutes` list, so the new route is auth-protected by default with zero middleware changes. |
+| REVIEW-02 | Timeline — reports + classification history + audit events, chronological | Confirmed schema/queries: `reports` (per-campaign, `ORDER BY created_at ASC`), `classifications` (per-campaign, `ORDER BY created_at DESC` for latest but ASC for timeline), `audit_events` (per-campaign, 4 canonical `event_type` values confirmed in `phishing-audit.ts` + `classify/route.ts`: `campaign_classified`, `remediation_approved`, `remediation_completed`, `campaign_marked_false_positive`). |
+| REVIEW-03 | Evidence — EML headers/URLs/attachments, body preview, blast radius w/ `unavailable` state | Confirmed exact `messages` column shapes from `phishing-eml-service.ts`'s actual INSERT statement (headers/urls/attachments/body_preview JSONB/TEXT shapes documented below). Confirmed blast radius MUST be fetched fresh (no persisted structured version exists) — see Summary. |
+| REVIEW-04 | Classification display — verdict/confidence/reasons/recommended actions | Confirmed exact `classifications` row shape and `recommended_actions` vocabulary (7 action types) from `campaign-classifier.ts`'s `mapVerdictToActions`. |
+| REVIEW-05 | Approve/remediate/mark-false-positive from the page, reflecting resulting state | Confirmed exact request/response shapes for all three routes (below) by reading `approve/route.ts`, `remediate/route.ts`, `mark-false-positive/route.ts`, and `remediation-service.ts` in full. |
+| REVIEW-06 | No relaxed/separate permission model — disabled/hidden not failed request | Confirmed `hasPermission()` (`lib/permissions.ts`) is a pure, isomorphic function (only imports `better-auth/plugins/access`) safe to call client-side, and confirmed the exact server-side gate (`requirePermission('phishing', 'approve'|'remediate')`) it must mirror. Confirmed NO existing page in the codebase calls `hasPermission()` client-side today (see Pitfall 4) — this phase establishes the first instance of that exact pattern, contrary to CONTEXT.md's assumption that a concrete analog exists. |
+</phase_requirements>
+
+## Architecture Patterns
+
+### System Architecture Diagram
+
+```
+Autotask ticket (LiveLink button)
+        |
+        v  (browser navigation, numeric AT ticket ID as path param)
+GET /phishing/tickets/{ticketId}   [Next.js page, 'use client']
+        |
+        |-- 1. fetch(`/api/phishing/tickets/${ticketId}/campaign`)  [NEW route]
+        |         -> SELECT id, campaign_id FROM reports WHERE ticket_id = $1
+        |         -> 404-shaped "not yet triaged" response if no report row (D-07)
+        |         -> { reportId, campaignId: null } if report exists but ungrouped (D-08)
+        |         -> { reportId, campaignId } on success
+        |
+        |-- 2. IF campaignId: fetch(`/api/phishing/campaigns/${campaignId}`)  [EXTENDED existing route]
+        |         -> bulk-fetch reports, messages, indicators (existing)
+        |         -> + remediation_actions (NEW field)
+        |         -> + audit_events (NEW field)
+        |         -> + full classifications row incl. reasons/recommended_actions/requires_approval (NEW field)
+        |         -> + server-side getBlastRadius() call keyed off primary report's real
+        |            sender/recipient/subject/date-window (NEW — fresh every load, see Pitfall 3)
+        |         -> + merged/sorted timeline array (reports + classifications + audit_events)
+        |
+        |-- 3. Client renders: ClassificationCard -> ActionAreaCard -> EvidenceCard + TimelineCard
+        |
+        |-- 4. Operator checks action(s) in ActionAreaCard -> POST /api/phishing/campaigns/{id}/approve
+        |         { actions: [{ actionType, params }] }  -> existing Phase 20 route, unchanged
+        |-- 5. Operator clicks Remediate -> POST /api/phishing/campaigns/{id}/remediate  (no body)
+        |-- 6. Operator clicks Mark false positive -> POST /api/phishing/campaigns/{id}/mark-false-positive
+        |         { reason? }
+        |
+        |-- 7. On any 2xx from 4/5/6: re-fetch step 2's endpoint (D-04, no optimistic mutation)
+        |
+        v
+   sonner toast (success/error) + re-rendered page from fresh server state
+```
+
+### Recommended Project Structure
+```
+app/
+├── phishing/
+│   ├── page.tsx                          # NEW — campaigns list (D-00)
+│   └── tickets/
+│       └── [ticketId]/
+│           └── page.tsx                  # NEW — REVIEW-01..06 review page
+├── api/
+│   └── phishing/
+│       ├── campaigns/
+│       │   ├── route.ts                  # EXTEND — add firstReportTicketId field
+│       │   └── [id]/
+│       │       └── route.ts              # EXTEND — add remediationActions/auditEvents/full classification/blastRadius/timeline
+│       └── tickets/
+│           └── [ticket_id]/
+│               └── campaign/
+│                   └── route.ts          # NEW — ticket->campaign resolver
+components/
+├── phishing/
+│   ├── classification-card.tsx           # NEW
+│   ├── action-area-card.tsx              # NEW
+│   ├── evidence-card.tsx                 # NEW
+│   ├── timeline-card.tsx                 # NEW
+│   └── url-list.tsx                      # NEW
+└── ui/
+    └── tooltip.tsx                       # NEW (shadcn add)
+```
+
+### Pattern 1: Ticket→campaign resolver (new route)
+**What:** A minimal, permission-gated lookup route that turns an Autotask numeric ticket ID into `{ reportId, campaignId }` (or an explicit not-yet-triaged / ungrouped signal), matching the existing `requirePermission` + UUID/number-validation idiom used by every other phishing route.
+**When to use:** Called once, first, by the ticket-scoped review page before it knows which campaign (if any) to fetch.
+**Example (shape — not the exact file, based on the existing `analyze/route.ts` idiom read in full):**
+```typescript
+// app/api/phishing/tickets/[ticket_id]/campaign/route.ts
+// Source: mirrors app/api/phishing/tickets/[ticket_id]/analyze/route.ts's
+// requirePermission + Number(ticket_id) validation pattern (read directly).
+export async function GET(
+  request: NextRequest,
+  { params }: { params: Promise<{ ticket_id: string }> }
+) {
+  const { error } = await requirePermission('phishing', 'read');
+  if (error) return error;
+
+  const { ticket_id } = await params;
+  const ticketId = Number(ticket_id);
+  if (!Number.isFinite(ticketId)) {
+    return NextResponse.json({ error: 'Invalid ticket_id' }, { status: 400 });
+  }
+
+  const res = await postgresClient.query<{ id: string; campaign_id: string | null; ticket_number: string | null }>(
+    `SELECT id::text, campaign_id::text, ticket_number FROM reports WHERE ticket_id = $1`,
+    [ticketId]
+  );
+  const report = res.rows[0];
+  if (!report) {
+    // D-07: no report row yet — page renders "Not yet triaged" empty state, not a 404.
+    return NextResponse.json({ found: false }, { status: 200 });
+  }
+  return NextResponse.json({
+    found: true,
+    reportId: report.id,
+    campaignId: report.campaign_id, // null => D-08 ungrouped-report path
+    ticketNumber: report.ticket_number,
+  });
+}
+```
+**Why `found: false` with 200, not a 404:** D-07 explicitly says "Not a 404" — the page must distinguish "route works, no data yet" (render empty state) from "route/campaign genuinely doesn't exist" (error state). A 404 status would force the client to disambiguate via response body inspection anyway, so a 200 with a `found` boolean is simpler and matches the page's own state machine (loading / not-yet-triaged / ungrouped / full-campaign / error) more directly than HTTP status alone can express.
+
+### Pattern 2: Extending the campaign-detail route safely
+**What:** Add four fields to the existing `GET /api/phishing/campaigns/[id]` response without touching any existing field.
+**When to use:** This IS the review page's main data source.
+**Confirmed safe because:** A repo-wide grep found no other consumer of this route besides itself — it is safe to add fields (additive change, no field removed/renamed).
+**Example — the four additions, each independently bolted onto the existing bulk-fetch pattern already in the route:**
+```typescript
+// Source: read directly from app/api/phishing/campaigns/[id]/route.ts (existing)
+// and remediation-service.ts / phishing-audit.ts / campaign-classifier.ts (existing).
+
+// 1. remediation_actions — same campaign_id filter idiom already used for classifications
+const remediationRes = await postgresClient.query<RemediationActionRow>(
+  `SELECT id::text, action_type, status, params, approved_by, approved_at::text
+     FROM remediation_actions WHERE campaign_id = $1 ORDER BY created_at ASC`,
+  [id]
+);
+
+// 2. audit_events — audit_events.campaign_id has NO FK constraint (plain UUID column,
+//    per migration 097) but is always populated by writeAuditEvent() with the same id.
+const auditRes = await postgresClient.query<AuditEventRow>(
+  `SELECT id::text, actor, event_type, payload, created_at::text
+     FROM audit_events WHERE campaign_id = $1 ORDER BY created_at ASC`,
+  [id]
+);
+
+// 3. full classifications row — existing query already selects verdict/confidence/summary;
+//    add reasons, recommended_actions, requires_approval (all already columns on the table)
+`SELECT id::text, verdict, confidence, summary, reasons, recommended_actions,
+        requires_approval, created_at::text
+   FROM classifications WHERE campaign_id = $1 ORDER BY created_at DESC`
+
+// 4. blastRadius — derive sender/recipient/subject/date-window from the EARLIEST
+//    report's parsed message + indicators (mirrors campaign-classifier.ts's
+//    gatherCampaignEvidence exactly — NOT triage-note-service.ts's empty-string bug,
+//    see Pitfall 3), then call getBlastRadius() fresh every request.
+```
+
+### Pattern 3: Deriving "completed on {date} by {approver}" without a schema change
+**What:** `remediation_actions` has `approved_by`/`approved_at` but no `completed_at`. Join against `audit_events` to recover the completion timestamp.
+**When to use:** In the extended detail route, when building the `remediationActions` array for display.
+**Example:**
+```typescript
+// After fetching remediationRes and auditRes (Pattern 2 above), in application code
+// (not SQL — small N, no need for a JOIN):
+const completedAtByActionId = new Map<string, string>();
+for (const event of auditRes.rows) {
+  if (event.event_type === 'remediation_completed') {
+    const actionId = (event.payload as { actionId?: string })?.actionId;
+    if (actionId) completedAtByActionId.set(actionId, event.created_at);
+  }
+}
+// remediationActions.map(a => ({ ...a, completedAt: completedAtByActionId.get(a.id) ?? null }))
+```
+This is possible because `remediateApprovedActions` (in `remediation-service.ts`, read in full) writes exactly one `remediation_completed` audit row per transitioned action with `payload: { actionId, actionType }` — confirmed by direct code read, not inference.
+
+### Pattern 4: Client-side permission gating (D-06/REVIEW-06) — first instance in this codebase
+**What:** Call `hasPermission(role, 'phishing', 'approve' | 'remediate')` from `lib/permissions.ts`, fed by `useSession()` from `lib/auth-client.ts`.
+**When to use:** Gating the three action buttons in `ActionAreaCard`, per D-06's explicit lock.
+**Important correction to CONTEXT.md's assumption:** CONTEXT.md's D-06/canonical-refs section states this pattern is "already safe to import client-side" and implies a concrete existing analog. A repo-wide grep for `hasPermission` found **zero** usages anywhere in `app/` or `components/` — this function has never been called client-side before in Pulse. The closest existing analogs (`components/rmm/rmm-dispatch-dialog.tsx`, `components/navigation/app-navigation.tsx`) both use a simpler direct string comparison instead:
+```typescript
+// Source: components/rmm/rmm-dispatch-dialog.tsx (existing, read directly)
+const { data: session } = useSession();
+const role = (session?.user as { role?: string } | undefined)?.role ?? 'user';
+const canExecute = role === 'admin' || role === 'super-admin';
+```
+CONTEXT.md's D-06 is still the correct decision to follow (it is an explicit locked decision, not up for debate, and `hasPermission()` genuinely is a pure/isomorphic function safe to import client-side — confirmed by reading `lib/permissions.ts` in full: it only imports `better-auth/plugins/access` and performs no I/O). The research finding is narrower: **there is no existing file to copy this exact call-shape from — this phase writes the first one.** Recommended shape, matching the confirmed `hasPermission(roleName, resource, action)` signature:
+```typescript
+import { useSession } from '@/lib/auth-client';
+import { hasPermission } from '@/lib/permissions';
+
+const { data: session } = useSession();
+const role = (session?.user as { role?: string } | undefined)?.role ?? 'user';
+const canApprove = hasPermission(role, 'phishing', 'approve');
+const canRemediate = hasPermission(role, 'phishing', 'remediate');
+```
+
+### Anti-Patterns to Avoid
+- **Passing empty-string sender/recipient to `getBlastRadius()`:** `triage-note-service.ts` (Phase 21) does exactly this (`sender: '', recipient: ''`) — see Pitfall 3. Do not copy that call site verbatim; copy `campaign-classifier.ts`'s `gatherCampaignEvidence` call site instead, which correctly derives sender from the `sender`-type indicator (falling back to the parsed message's `from.email`) and recipient from the report's joined `requester_contact_id` email.
+- **Rendering `messages.headers` fields as raw HTML or calling `dangerouslySetInnerHTML` on `body_preview`:** `body_preview` is plain text (EVID-04) — always render inside a `<pre>` with `whitespace-pre-wrap`, never interpret as HTML even though the source email may have been HTML.
+- **Making extracted URLs clickable in any form** (`<a href>`, `onClick` navigation, or an accidental `<Link>` wrapper) — D-09 requires plain inert text with copy-to-clipboard only.
+- **Building a second campaign-ID-scoped review page** for the list page's row-click target — REVIEW-01 mandates exactly one ticket-scoped review page; the list page must navigate to `/phishing/tickets/{firstReportTicketId}`, not a new `/phishing/campaigns/{id}` page.
+
+## Don't Hand-Roll
+
+| Problem | Don't Build | Use Instead | Why |
+|---------|-------------|-------------|-----|
+| Client-side role/permission check | A bespoke `role === 'admin'` conditional (the existing codebase pattern) | `hasPermission(role, 'phishing', action)` from `lib/permissions.ts` | D-06 explicitly locks this — it's the exact function the API routes already enforce server-side, so client and server can never drift out of sync |
+| Server-paginated list table | A hand-rolled `<table>` with manual pagination state | `components/admin/DataTable.tsx` (confirmed `Column<TData>[]` + `totalCount`/`page`/`pageSize`/`onPageChange`/`onRowClick` props by reading the file) | Existing reusable component already used by 10+ `app/admin/data-browser/*` pages against the same `limit`/`offset`/`total` API shape `GET /api/phishing/campaigns` already returns |
+| Disabled-button tooltip explanations | Custom `title` attribute or a hand-rolled popover | shadcn `tooltip` (new addition this phase) | UI-SPEC's Registry Safety table already scoped this as the one net-new primitive; don't invent a second one |
+| Copy-to-clipboard | `document.execCommand('copy')` or a new dependency | `navigator.clipboard.writeText(url)` (already specified in UI-SPEC) | Native browser API, zero dependencies, already the UI-SPEC's prescribed implementation |
+
+**Key insight:** Every "hard part" of this phase (permission enforcement, remediation state machine, audit trail, blast-radius abstraction, EML parsing) was already built and hardened in Phases 15-21. This phase's only genuinely new logic is (a) the ticket→campaign resolution query, (b) the timeline merge/sort, and (c) the client-side default-param derivation table for the 7 action types (UI-SPEC Action Area Spec) — everything else is composition of existing services.
+
+## Common Pitfalls
+
+### Pitfall 1: Assuming `classifications.reasons` contains structured blast-radius data
+**What goes wrong:** A planner reads CONTEXT.md's "read persisted `reasons`" option as a real, cheaper alternative to a fresh Mimecast call, and specs the Evidence card's Blast Radius tab to parse `reasons` for delivered/held/rejected counts.
+**Why it happens:** `reasons` is JSONB and does sometimes mention Mimecast ("Mimecast blast-radius data unavailable...") which looks superficially like it could carry the full result.
+**How to avoid:** Confirmed by reading `campaign-classifier.ts`'s `computeConfidence()` in full — `reasons` only ever contains short natural-language strings, and only adds a Mimecast-related string when blast radius is *unavailable*. The full `BlastRadiusResult` object (matched/delivered/held/rejected/clicked/perRecipient) is never persisted anywhere in the schema. The extended detail route must call `getBlastRadius()` fresh, every request.
+**Warning signs:** If a plan's `must_haves` says "parse blast radius from `classifications.reasons`," that plan will fail at execution — there's no structured data there to parse.
+
+### Pitfall 2: Missing `remediation_actions.completed_at`
+**What goes wrong:** A plan assumes a `completed_at` column exists (many other tables in this codebase have one — `sync_history`, `pipeline_engine` executions, `rmm_executions`, etc. — so it's a reasonable but wrong assumption) and writes `SELECT completed_at FROM remediation_actions` or similar, which will fail at the SQL layer (undefined column).
+**Why it happens:** `completed_at` is such a common column pattern elsewhere in the codebase (confirmed via grep across a dozen other tables) that its absence here is a genuine outlier.
+**How to avoid:** Derive it from `audit_events` (Pattern 3 above) — confirmed viable because `remediateApprovedActions` writes one `remediation_completed` audit row per transitioned action with `payload.actionId` matching the `remediation_actions.id`.
+**Warning signs:** A 500 error mentioning `column "completed_at" does not exist` at execution time.
+
+### Pitfall 3: Copying `triage-note-service.ts`'s empty-string blast-radius call
+**What goes wrong:** Phase 21's `generateAndPostTriageNote()` calls `getBlastRadius({ sender: '', recipient: '', subject: primaryReport.title ?? '', dateWindow: {...} })` — passing empty strings for sender and recipient. `getBlastRadius`'s own type signature marks both as required (non-optional `string`), and its internal fan-out (`searchDeliveredMessages({ to, from, subject, ... })`) will run against Mimecast with blank `to`/`from` filters, which will not usefully scope the result to this campaign's actual sender/recipient. If this phase's Evidence card copies that call site verbatim (since it's the most recent precedent), the blast-radius tab will show meaningless/empty data for essentially the same reason it would with sender/recipient omitted entirely.
+**Why it happens:** `triage-note-service.ts`'s `ReportRow` interface only selects `title`, not the message headers or requester email needed to populate sender/recipient correctly — it's a narrower query than `campaign-classifier.ts`'s `gatherCampaignEvidence`, which DOES derive real sender (from a `sender`-type indicator or the parsed message's `from.email`) and real recipient (the report's joined `requester_contact_id` email).
+**How to avoid:** When building the extended detail route's blast-radius call, copy `campaign-classifier.ts`'s `gatherCampaignEvidence` sender/recipient derivation logic (join `messages`/`indicators` for the earliest report, plus the existing `contacts` join for `requester_email` already present in the current `campaigns/[id]/route.ts`), not `triage-note-service.ts`'s.
+**Warning signs:** Blast-radius tab always shows `matched: 0, delivered: 0` even when Mimecast is configured and genuinely has data for this campaign.
+
+### Pitfall 4: Assuming an existing client-side `hasPermission()` call site to copy
+**What goes wrong:** A plan says "mirror the existing pattern at X" for D-06's client-side gate, but no such X exists (see Architecture Pattern 4) — the plan's verification step would look for a reference implementation that isn't there.
+**Why it happens:** CONTEXT.md's canonical-refs section states the pattern is reused/already-safe without flagging that "already safe to import" (true) and "an existing call site to copy" (not true) are different claims.
+**How to avoid:** Treat Pattern 4 above as the reference implementation — it's synthesized from the confirmed-safe `hasPermission()` signature plus the confirmed `useSession()` usage pattern from `rmm-dispatch-dialog.tsx`, not copied from a single existing file.
+**Warning signs:** None at runtime — this is a planning-accuracy pitfall, not a runtime bug. Only matters if a plan's task literally says "copy the pattern from file Y" and Y doesn't contain it.
+
+### Pitfall 5: `audit_events.campaign_id` has no FK constraint
+**What goes wrong:** Assuming a `JOIN audit_events ON audit_events.campaign_id = campaigns.id` is guaranteed referentially valid the way `classifications.campaign_id` (which also lacks an explicit FK per the migration, actually — worth double-checking) is expected to behave.
+**Why it happens:** Migration 097 declares `audit_events.campaign_id UUID` with NO `REFERENCES campaigns(id)` constraint (unlike `reports.campaign_id`, `classifications.campaign_id`, and `remediation_actions.campaign_id`, which likewise have no FK reference to campaigns either, actually — re-checking migration 097: none of `classifications`, `remediation_actions`, or `audit_events` declare a `REFERENCES campaigns(id)` FK; only `reports.campaign_id REFERENCES campaigns(id)` does).
+**How to avoid:** This is a pre-existing schema property (not introduced by this phase) and in practice is safe because every writer (`writeAuditEvent`, `classifyCampaign`, `approveRemediationActions`, etc.) is only ever called with a `campaignId` that was already validated against the `campaigns` table (every route 404s on an unknown UUID before calling the service). No action needed for this phase beyond being aware the join is trusted-by-convention, not DB-enforced.
+**Warning signs:** None expected in practice — noted for completeness since a planner reading raw DDL might otherwise flag it as a modeling gap requiring a fix.
+
+## Code Examples
+
+### Full request/response shapes for approve/remediate/mark-false-positive (confirmed by direct code read)
+
+```typescript
+// POST /api/phishing/campaigns/{id}/approve
+// Body:
+interface ApproveRequestBody {
+  actions: Array<{ actionType: string; params?: Record<string, unknown> }>;
+}
+// Response (200): ApprovedRemediationAction[]
+interface ApprovedRemediationAction {
+  id: string;
+  campaignId: string;
+  actionType: string;
+  status: 'approved';
+  approvedBy: string | null;
+}
+// Errors: 400 (bad UUID / empty actions array / action not in recommended_actions),
+//         404 (campaign not found), 409 (RemediationConflictError — not used by
+//         approve currently, but the route catches it defensively)
+
+// POST /api/phishing/campaigns/{id}/remediate
+// Body: none
+// Response (200):
+interface RemediateResult {
+  campaignId: string;
+  actions: Array<{
+    id: string;
+    actionType: string;
+    status: 'completed';
+    alreadyCompleted: boolean; // true when idempotent re-run found it already completed
+  }>;
+}
+// Errors: 400 (bad UUID / zero remediation_actions rows exist at all), 404 (campaign not found)
+
+// POST /api/phishing/campaigns/{id}/mark-false-positive
+// Body (optional): { reason?: string }
+// Response (200):
+interface MarkFalsePositiveResult {
+  campaignId: string;
+  status: 'false_positive';
+  auditEventId: string;
+}
+// Errors: 400 (bad UUID), 404 (campaign not found),
+//         409 (approved/completed remediation already exists — D-04 guard)
+```
+
+### Confirmed `messages` row shape (exact persisted JSONB structure)
+```typescript
+// Source: lib/services/phishing-eml-service.ts (read directly, the actual INSERT)
+// messages.headers JSONB:
+interface MessageHeaders {
+  from: { displayName: string | null; email: string | null; domain: string | null };
+  replyTo: string | null;
+  returnPath: string | null;
+  to: string[];
+  cc: string[];
+  subject: string | null;
+  date: string | null;
+  messageId: string | null;
+  receivedChain: string[];
+  authResults: { spf?: string; dkim?: string; dmarc?: string };
+  authResultsOriginal: { spf?: string; dkim?: string; dmarc?: string } | null;
+}
+// messages.urls JSONB: string[]
+// messages.attachments JSONB: Array<{ filename: string | null; contentType: string | null; size: number; checksum: string | null; related: boolean }>
+// messages.body_preview: TEXT (plain text, max 500 chars, EVID-04 truncation already applied)
+```
+
+### Confirmed recommended-action vocabulary (all 7 types, exhaustive)
+```typescript
+// Source: lib/services/campaign-classifier.ts mapVerdictToActions() (read directly)
+// SPAM -> ['no_action']
+// UNWANTED -> ['warn_user']
+// THREAT (base) -> ['block_sender', 'purge_message']
+// THREAT (evidence.clicked > 0) -> adds ['reset_password', 'isolate_endpoint', 'disable_forwarding_rule']
+// DESTRUCTIVE_ACTIONS (always requires_approval:true): block_sender, purge_message, reset_password, isolate_endpoint
+// NOT in DESTRUCTIVE_ACTIONS: no_action, warn_user, disable_forwarding_rule
+```
+This confirms the UI-SPEC's 7-row default-params table (Action Area Spec) is exhaustive and matches the real vocabulary exactly — no 8th action type exists to plan for.
+
+## State of the Art
+
+Not applicable in the traditional sense (no external library version churn to track) — the "state of the art" here is Phase 20/21's own precedent, which this phase should match:
+
+| Old Approach (would be wrong for this phase) | Current Approach (this phase should follow) | When Changed | Impact |
+|--------------|------------------|--------------|--------|
+| Optimistic local state mutation after approve/remediate | Refetch campaign detail from server (D-04) | Locked by CONTEXT.md this session | Guarantees the page always reflects server truth, no client/server drift possible |
+| A second campaign-ID-scoped page for list-row navigation | Single ticket-scoped review page, list row navigates via `firstReportTicketId` | Locked by CONTEXT.md/UI-SPEC this session | Exactly one review-page implementation to maintain |
+
+**Deprecated/outdated:** N/A — no prior version of this UI exists to deprecate.
+
+## Assumptions Log
+
+| # | Claim | Section | Risk if Wrong |
+|---|-------|---------|---------------|
+| A1 | Autotask LiveLink supplies the numeric AT ticket ID (matching `tickets.id`/`reports.ticket_id`) as the dynamic URL segment, not the human-readable `ticket_number` | Architecture Patterns, Pattern 1 | If LiveLink actually supplies `ticket_number` (e.g. "T20260101.0001"), the resolver route's `Number(ticket_id)` parse would fail on every request. This is inferred by consistency with the existing `/api/phishing/tickets/{ticket_id}/analyze` route (Phase 18, already shipped and presumably already validated against real Autotask LiveLink behavior in production) rather than independently verified against live Autotask LiveLink configuration docs in this session — no Autotask LiveLink admin-console access was available to confirm directly. |
+| A2 | Server-merging the timeline (reports + classifications + audit_events into one sorted array) inside the extended API route is preferable to a client-side merge | Architecture Patterns, Alternatives Considered | Low risk — CONTEXT.md explicitly leaves this to planner's discretion either way; recommendation is a preference, not a hard requirement. If planner chooses client-side merge instead, no correctness issue, just a different code-location tradeoff. |
+
+**If this table is empty:** N/A — see above, two low-to-moderate-risk assumptions logged.
+
+## Open Questions
+
+1. **Does Autotask LiveLink actually pass the numeric ticket ID, or the ticket number?**
+   - What we know: The existing (already-shipped, Phase 18) `/api/phishing/tickets/{ticket_id}/analyze` route treats its `ticket_id` path param as the numeric `tickets.id`/AT entity ID, via `Number(ticket_id)`. ROADMAP.md and CONTEXT.md both describe LiveLink as supplying "the ticket ID" without specifying numeric-ID vs. ticket-number.
+   - What's unclear: No direct access to the Autotask LiveLink admin configuration UI was available this session to confirm which field a configured LiveLink button would interpolate into a target URL.
+   - Recommendation: Follow the existing `analyze` route's precedent (numeric AT ID) for consistency — if it turns out wrong in a later integration/manual test, the fix is a one-line change (parse `ticket_number` string and look up via `reports.ticket_number` instead of `reports.ticket_id`, since `reports.ticket_number` is already a stored column). Flag this as a manual verification item for whoever configures the actual LiveLink button in Autotask (likely outside this phase's automated test surface entirely, since it requires a real Autotask tenant).
+
+## Environment Availability
+
+Skip — this phase has no new external dependencies. All existing integrations (Postgres, Mimecast client, Autotask client) are already configured and used by prior phases; this phase adds no new env vars, no new services, no new CLI tools.
+
+## Validation Architecture
+
+### Test Framework
+| Property | Value |
+|----------|-------|
+| Framework | vitest 4.1.5 |
+| Config file | `/opt/stacks/pulse/vitest.config.ts` |
+| Quick run command | `npm test -- lib/services/campaign-classifier.test.ts` (or the specific new test file) |
+| Full suite command | `npm test` |
+
+**Confirmed constraint:** `vitest.config.ts`'s `test.include` is `['lib/**/*.test.ts']` only — it does NOT include `app/**` or `components/**`. This means React page/component code and Next.js API route handlers in this phase have **no automated test coverage under the current config** — consistent with CLAUDE.md's "Other parts of the codebase have no tests... type-check is the only safety net." Any genuinely new *pure logic* this phase introduces (see below) should be extracted into a `lib/services/*.ts` file specifically so it CAN be unit-tested; UI composition and route wiring fall back to `npx tsc --noEmit --pretty` as the safety net, matching existing project convention.
+
+### Phase Requirements → Test Map
+| Req ID | Behavior | Test Type | Automated Command | File Exists? |
+|--------|----------|-----------|-------------------|-------------|
+| REVIEW-01 | Ticket→campaign resolver returns correct `{found, reportId, campaignId}` shape for: no report, ungrouped report, grouped campaign | unit (if resolver query logic is extracted to a testable `lib/services/*.ts` helper) | `npx vitest run lib/services/ticket-campaign-resolver.test.ts` | ❌ Wave 0 — recommend extracting the SQL lookup into a small `lib/services/phishing-ticket-resolver.ts` function specifically so this is unit-testable against a test DB/mock, rather than leaving the logic inline in the route handler (which vitest.config.ts's include pattern cannot reach) |
+| REVIEW-04 | Default-param derivation table (7 action types -> client-side default params) | unit | `npx vitest run lib/services/remediation-default-params.test.ts` (if extracted) | ❌ Wave 0 — recommend extracting UI-SPEC's Action Area default-param derivation logic (Input/Textarea prefill values) into a pure, testable function rather than inline component logic, since this is the one genuinely new piece of business logic in this phase |
+| REVIEW-02 | Timeline merge/sort (reports + classifications + audit_events, chronological) | unit (if server-merged) | `npx vitest run lib/services/phishing-timeline.test.ts` (if extracted) | ❌ Wave 0 — only applicable if planner chooses the server-merge approach (Architecture Alternatives Considered); if client-merged instead, this becomes untested UI logic same as the rest of the page |
+| REVIEW-03, REVIEW-05, REVIEW-06 | Page rendering, action button gating/disabling, evidence display | manual-only | — (justification: no component/page test infra exists in this codebase; `vitest.config.ts` does not include `app/**`/`components/**`) | — |
+
+### Sampling Rate
+- **Per task commit:** `npx tsc --noEmit --pretty` (type-check, matches CLAUDE.md's stated safety net for untested code) + `npm test -- <any newly-added lib/services test file>` if one exists for that task
+- **Per wave merge:** `npm test` (full suite) + `npx tsc --noEmit --pretty`
+- **Phase gate:** Full suite green + type-check green before `/gsd:verify-work`; manual click-through of both new pages (list page + ticket-scoped review page in all four states: full-campaign, D-07 not-yet-triaged, D-08 ungrouped, load-error) since REVIEW-02/03/05/06 have no automated coverage
+
+### Wave 0 Gaps
+- [ ] Decide whether to extract ticket→campaign resolution SQL into a testable `lib/services/phishing-ticket-resolver.ts` (recommended) vs. leaving it inline in the route handler (untestable under current vitest config)
+- [ ] Decide whether to extract the 7-action-type default-param-derivation table (UI-SPEC's Action Area Spec) into a pure `lib/services/remediation-default-params.ts` function (recommended, since this is the phase's one piece of genuinely new business logic worth unit-testing) vs. inline component logic
+- [ ] If server-merging the timeline: extract that merge/sort into a testable pure function rather than inline in the route handler
+- [ ] No framework install needed — vitest already configured and passing for the rest of the codebase
+
+## Security Domain
+
+### Applicable ASVS Categories
+
+| ASVS Category | Applies | Standard Control |
+|---------------|---------|-----------------|
+| V2 Authentication | yes (indirect) | Existing Better Auth session cookie, enforced by `middleware.ts` — no new auth surface introduced by this phase (REVIEW-01 explicitly forbids a separate token/query-param scheme) |
+| V3 Session Management | yes (indirect) | Unchanged — reuses existing Better Auth session; no new session state introduced |
+| V4 Access Control | yes | `requirePermission('phishing', 'read'|'analyze'|'approve'|'remediate')` server-side (existing, unchanged) + client-side `hasPermission()` mirror for UX only (REVIEW-06) — server remains sole enforcement point, confirmed no route in this phase's plan bypasses `requirePermission` |
+| V5 Input Validation | yes | UUID-shape regex validation (`UUID_RE`) already the established idiom for every existing `[id]` route — the new ticket→campaign resolver route must validate `ticket_id` as `Number.isFinite()` matching the existing `analyze/route.ts` idiom exactly |
+| V6 Cryptography | no | Not applicable — no new crypto/hashing/secret handling in this phase |
+
+### Known Threat Patterns for this stack
+
+| Pattern | STRIDE | Standard Mitigation |
+|---------|--------|---------------------|
+| Reflected/stored XSS via rendered email content (attacker-controlled `.eml` body/headers) | Tampering / Information Disclosure | Never `dangerouslySetInnerHTML` on `body_preview` (always plain `<pre>` text); never render extracted URLs as clickable `<a href>` (D-09 — inert copy-only text, this phase's explicit stricter posture); React's default JSX text-escaping already protects header field rendering as long as no field is passed through `dangerouslySetInnerHTML` anywhere |
+| IDOR — an operator without `phishing:approve`/`remediate` calling the write routes directly (bypassing the UI) | Elevation of Privilege | Already fully mitigated server-side by the existing `requirePermission('phishing', 'approve'|'remediate')` gates in all three write routes (confirmed by direct code read) — this phase's client-side gating (REVIEW-06) is a UX improvement only, not a new security boundary; the security boundary already exists and is unchanged |
+| Campaign-UUID enumeration via the extended detail route | Information Disclosure | Already mitigated — `requirePermission('phishing', 'read')` gates the entire route; a UUID guess without `phishing:read` permission still 401/403s before any query runs |
+| CSRF on the three write routes (approve/remediate/mark-false-positive) | Tampering | Out of scope for this phase — same-origin `fetch()` calls from a Better-Auth-session-cookie'd page, matching every other write route in this codebase's existing (unaudited-by-this-phase) CSRF posture; no change introduced or required here |
+
+## Sources
+
+### Primary (HIGH confidence — direct code reads this session)
+- `/opt/stacks/pulse/app/api/phishing/campaigns/[id]/route.ts` — current GET shape, bulk-fetch pattern, no-other-consumer confirmation basis
+- `/opt/stacks/pulse/app/api/phishing/campaigns/route.ts` — list endpoint, limit/offset/total shape
+- `/opt/stacks/pulse/app/api/phishing/campaigns/[id]/approve/route.ts`, `remediate/route.ts`, `mark-false-positive/route.ts`, `classify/route.ts`, `triage-note/route.ts` — full request/response/error shapes
+- `/opt/stacks/pulse/app/api/phishing/tickets/[ticket_id]/analyze/route.ts` — confirms `ticket_id` param = `tickets.id` numeric AT ID
+- `/opt/stacks/pulse/lib/services/remediation-service.ts` — full approve/remediate/mark-false-positive orchestration logic, confirms no `completed_at` column and the `remediation_completed` audit payload shape
+- `/opt/stacks/pulse/lib/services/mimecast-blast-radius.ts` — `getBlastRadius()` full signature, confirms `BlastRadiusResult` is never persisted by this module (D-03 "ephemeral" doc-comment)
+- `/opt/stacks/pulse/lib/services/triage-note-service.ts` — Phase 21 precedent, confirms the empty-string sender/recipient bug (Pitfall 3)
+- `/opt/stacks/pulse/lib/services/campaign-classifier.ts` — confirms `reasons`/`recommended_actions` never carry structured blast-radius data, confirms exhaustive 7-action vocabulary, confirms correct sender/recipient derivation pattern to copy instead
+- `/opt/stacks/pulse/lib/services/phishing-audit.ts` — confirms exhaustive `event_type` vocabulary (4 values) and `writeAuditEvent` shape
+- `/opt/stacks/pulse/lib/services/phishing-eml-service.ts` — confirms exact `messages.headers`/`urls`/`attachments`/`body_preview` persisted shapes
+- `/opt/stacks/pulse/migrations/097_phishing_triage_schema.sql`, `098_phishing_sweep_schedule.sql`, `099_indicators_metadata.sql` — full schema, confirms no `completed_at` column on `remediation_actions`, confirms no FK from `classifications`/`remediation_actions`/`audit_events` to `campaigns`
+- `/opt/stacks/pulse/migrations/001_initial_schema.sql` — confirms `tickets.id BIGINT PRIMARY KEY` + `ticket_number VARCHAR` distinction, confirms `contacts.email_address` column
+- `/opt/stacks/pulse/lib/permissions.ts` — confirms `hasPermission()` signature, isomorphic import safety, and the full role/permission matrix (`phishing: ["read","analyze","approve","remediate"]` for admin/super-admin, `["read"]` only for `user`)
+- `/opt/stacks/pulse/lib/auth-client.ts` — confirms `useSession()` export shape
+- `/opt/stacks/pulse/lib/auth-utils.ts` — confirms `requireAuth()`/`requirePermission()` server-side shapes
+- `/opt/stacks/pulse/components/rmm/rmm-dispatch-dialog.tsx`, `components/navigation/app-navigation.tsx` — confirms the actual (non-`hasPermission`) client-side role-check precedent in this codebase today
+- `/opt/stacks/pulse/components/admin/DataTable.tsx` — confirms `Column<TData>`/`DataTableProps` shape for the campaigns list page
+- `/opt/stacks/pulse/components/ui/status-badge.tsx`, `empty-state.tsx`, `skeleton-helpers.tsx` — confirms exact prop shapes for reused primitives
+- `/opt/stacks/pulse/middleware.ts` — confirms `/phishing` is absent from `publicRoutes`, so default auth-redirect behavior applies
+- `/opt/stacks/pulse/vitest.config.ts`, `package.json` — confirms test include pattern (`lib/**/*.test.ts` only) and `npm test` script
+- `/opt/stacks/pulse/.planning/config.json` — confirms `nyquist_validation: true`, `ui_phase: true`, `research: false` (orchestrator-level; this file itself is the research output regardless)
+- Repo-wide `grep -rl "hasPermission"` and `grep -rl "useSession"` across `app/` and `components/` — confirms zero existing client-side `hasPermission()` call sites (Pitfall 4)
+
+### Secondary (MEDIUM confidence)
+- None — every claim in this research was verified against the actual repo code, not external documentation or web search, since this phase involves zero new external libraries/services.
+
+### Tertiary (LOW confidence)
+- Assumption A1 (Autotask LiveLink's dynamic-ID semantics) — see Assumptions Log and Open Questions; based on internal consistency with an already-shipped route, not independently confirmed against Autotask's LiveLink configuration UI.
+
+## Metadata
+
+**Confidence breakdown:**
+- Standard stack: HIGH — zero new dependencies, one new shadcn primitive from the already-configured official registry
+- Architecture: HIGH — every route/table/service this phase touches was read in full this session; the three CONTEXT.md-deferred decisions all have evidence-backed answers, not guesses
+- Pitfalls: HIGH — all five pitfalls are drawn from direct code reads (missing column, empty-string bug in a real precedent file, absent client-side pattern), not speculation
+
+**Research date:** 2026-07-16
+**Valid until:** 30 days (stable — no external library churn risk; codebase-internal findings remain valid until Phase 20/21 services are modified, which is out of this phase's scope)