docs(23): add phase verification report (gaps found)

.planning/phases/23-classification-disposition-per-client-automation-gate/23-VERIFICATION.md

@@ -0,0 +1,151 @@
+---
+phase: 23-classification-disposition-per-client-automation-gate
+verified: 2026-07-16T21:20:00Z
+status: gaps_found
+score: 17/18 must-haves verified
+overrides_applied: 0
+gaps:
+  - truth: "auto_report auto-posts ONLY the acknowledge_user thank-you note for USER_AWARENESS verdicts — no other verdict/action is auto-posted or auto-approved"
+    status: partial
+    reason: "The action-type scoping is correct (only acknowledge_user is ever auto-posted, gated to USER_AWARENESS), but there is no idempotency guard before the post. runGatedPhishingStages() in lib/services/webhook-service.ts fires on every ticket-create webhook that lands in an existing campaign (groupReportIntoCampaign returns the same campaignId for created:false merges), and generateAndPostAcknowledgment() unconditionally re-queries every report linked to the campaign and re-posts the customer-visible note to all of their tickets on every pass. Since a phishing 'campaign' is by definition usually multiple employees reporting the same email, any company with auto_classify+auto_report enabled will duplicate (N-fold, over N additional reports) a customer-visible note on already-thanked ticket threads. This is CR-01 from the phase's own code review (23-REVIEW.md) and is unresolved in the current code."
+    artifacts:
+      - path: "lib/services/webhook-service.ts"
+        issue: "runGatedPhishingStages (lines 522-567) calls generateAndPostAcknowledgment(campaignId) with no check against remediation_actions or any other record of a prior auto-post for this campaign"
+      - path: "lib/services/triage-note-service.ts"
+        issue: "generateAndPostAcknowledgment (lines 212-251) queries ALL reports for the campaign_id and posts to all of their tickets every time it's called — no filter for already-notified tickets"
+    missing:
+      - "An idempotency check before the auto-post call in runGatedPhishingStages — e.g. query remediation_actions (or a new campaigns.acknowledge_user_posted_at column) for an existing acknowledge_user record for this campaignId, and skip the post if one exists"
+      - "A persisted record of the auto-post (remediation_actions row + audit_events row) so the record exists to check against on the next pass, and so the Action Area/Timeline UI reflect that the note was already sent (WR-01 in 23-REVIEW.md)"
+---
+
+# Phase 23: Classification Disposition + Per-Client Automation Gate Verification Report
+
+**Phase Goal:** Add a dedicated "User Awareness" classification disposition (`USER_AWARENESS` verdict + non-destructive `acknowledge_user` action that posts a customer-visible thank-you note), surface it distinctly in the review UI, add a per-Autotask-company automation-gate table + admin UI (three opt-in booleans: auto_parse/auto_classify/auto_report), and wire that gate into the Autotask webhook path so opted-in companies get an automatic parse→classify→acknowledge pipeline while every other action/verdict still requires manual approval.
+
+**Verified:** 2026-07-16T21:20:00Z
+**Status:** gaps_found
+**Re-verification:** No — initial verification
+
+## Goal Achievement
+
+### Observable Truths
+
+| # | Truth | Status | Evidence |
+|---|-------|--------|----------|
+| 1 | A campaign whose classifier reasoning matches the phishing-simulation-vendor allowlist is classified USER_AWARENESS, not UNWANTED | VERIFIED | `lib/services/campaign-classifier.ts:458-463` — `isSimulation` check short-circuits to `verdict = 'USER_AWARENESS'` before THREAT/SPAM/UNWANTED evaluation |
+| 2 | The USER_AWARENESS verdict recommends exactly the acknowledge_user action | VERIFIED | `mapVerdictToActions()` `case 'USER_AWARENESS': return ['acknowledge_user'];` (`campaign-classifier.ts:185-186`) |
+| 3 | acknowledge_user is NOT treated as destructive (requires_approval stays false for it) | VERIFIED | `DESTRUCTIVE_ACTIONS` set (`campaign-classifier.ts:153-158`) contains only `block_sender`/`purge_message`/`reset_password`/`isolate_endpoint` — `acknowledge_user` excluded, so `computeRequiresApproval` returns false |
+| 4 | acknowledge_user posts a customer-visible Autotask ticket note (noteType 18) with an appreciative thank-you body | VERIFIED | `generateAndPostAcknowledgment()` (`triage-note-service.ts:212-251`) posts `TicketNotes` with `noteType: 18` and a fixed appreciative body string |
+| 5 | Manual path: an approved acknowledge_user action posts a real note via generateAndPostAcknowledgment; the 7 other action types remain simulated status-only transitions | VERIFIED | `remediation-service.ts:217-233` — `shouldPostAcknowledgment` only true for rows transitioned `approved`→`completed` this pass (`alreadyCompleted === false`); other action types only get a status UPDATE + audit_events row (lines 194-211) |
+| 6 | A campaign classified USER_AWARENESS renders in ClassificationCard with a distinct badge color | VERIFIED | `classification-card.tsx:34` — `USER_AWARENESS: 'bg-emerald-500/15 text-emerald-600'` (distinct from UNWANTED's amber) |
+| 7 | acknowledge_user renders with a human-readable label wherever action labels appear | VERIFIED | `'Acknowledge user'` present in `classification-card.tsx:45` and `action-area-card.tsx:75` |
+| 8 | In the manual review UI, acknowledge_user appears as a normal checkbox + Approve action, no gate-check logic | VERIFIED | `action-area-card.tsx:134-139` — `case 'acknowledge_user'` mirrors `no_action`'s no-params template, no gate lookup in this file |
+| 9 | A USER_AWARENESS timeline entry renders with a distinct tint and does not crash the review page | VERIFIED | `timeline-card.tsx:22-26` (`VERDICT_TINT` 4th key) + line 98 (`(verdict && VERDICT_TINT[verdict]) \|\| 'bg-muted-foreground...'` defensive fallback before `.split()`) |
+| 10 | A phishing_automation_gate table exists keyed by company_id with three booleans defaulting to false | VERIFIED | `migrations/100_phishing_automation_gate.sql` — confirmed applied to the live `pulse-postgres` container (`\d phishing_automation_gate` shows all 3 booleans `NOT NULL DEFAULT false`) |
+| 11 | GET /api/admin/phishing-automation returns every company with its three gate flags, defaulting absent rows to all-false | VERIFIED | `app/api/admin/phishing-automation/route.ts` — `LEFT JOIN phishing_automation_gate` + `COALESCE(pag.auto_*, false)`, `requireAdmin()` gated |
+| 12 | PATCH /api/admin/phishing-automation/{companyId} upserts the three flags with actor+timestamp stamping, admin-gated | VERIFIED | `app/api/admin/phishing-automation/[companyId]/route.ts` — `requireAdmin()`, `INSERT ... ON CONFLICT (company_id) DO UPDATE`, `updated_by`/`updated_at` stamped |
+| 13 | A company with no row reads as all three stages OFF | VERIFIED | Same COALESCE pattern in both the list route and `getCompanyAutomationGate()`; migration comment states this explicitly |
+| 14 | REQUIREMENTS.md documents all six phase-23 requirement IDs with Traceability rows | VERIFIED | `.planning/REQUIREMENTS.md:142-165` (requirement descriptions) and `:239-244` (Traceability rows, all listed as "Phase 23 / Pending" — rows exist as required by the must-have; status text is a separate housekeeping concern, not blocking) |
+| 15 | An admin can open /admin/phishing-automation and see a searchable/filterable company table with 3 independent Switch toggles per row, reachable from /admin index | VERIFIED | `app/admin/phishing-automation/page.tsx` (266 lines, search/type filter chrome, 3 `<Switch>` cells lines 233-252) + `app/admin/page.tsx:250` tile linking to it. Human-verify checkpoint for this task was approved live against a rebuilt production container per plan 23-04's SUMMARY. |
+| 16 | getCompanyAutomationGate returns all-false for a company with no gate row and the stored values when a row exists | VERIFIED | `lib/services/phishing-automation-gate.ts` + `phishing-automation-gate.test.ts` (4 passing tests: no-row, row-exists, null companyId, NaN companyId) |
+| 17 | On a phishing-flagged webhook, after always-on detect+group, the pipeline runs parse→classify→acknowledge gated per-stage by the company's gate flags | VERIFIED | `webhook-service.ts:499-567` — `triggerPhishingDetection` always runs detection+grouping unconditionally, then calls `runGatedPhishingStages()` which gates each of the 3 stages independently on `gate.autoParse`/`autoClassify`/`autoReport` |
+| 18 | auto_report auto-posts ONLY the acknowledge_user note for USER_AWARENESS verdicts, with no duplicate-note risk on repeat webhook passes for the same campaign | **FAILED** | Action-type/verdict scoping is correct (`webhook-service.ts:560` gates on `verdict === 'USER_AWARENESS'` and calls nothing else), but there is **no idempotency guard**. `generateAndPostAcknowledgment()` (`triage-note-service.ts:213-217`) re-queries **every** report on the campaign and re-posts the note to all of their tickets on every invocation. Because `groupReportIntoCampaign` returns the same `campaignId` for both new-campaign and merged-into-existing-campaign cases, and `runGatedPhishingStages` fires on every ticket-create webhook, a company with `autoClassify`+`autoReport` on will re-send the customer-visible thank-you note to already-notified tickets every time a new report joins the same campaign (the normal shape of "campaign" data). This is CR-01 from `23-REVIEW.md`, confirmed present and unresolved by direct code inspection. |
+
+**Score:** 17/18 truths verified
+
+### Required Artifacts
+
+| Artifact | Expected | Status | Details |
+|----------|----------|--------|---------|
+| `lib/services/campaign-classifier.ts` | USER_AWARENESS verdict + action mapping | VERIFIED | Contains `USER_AWARENESS` literal, switch arm, DESTRUCTIVE_ACTIONS exclusion |
+| `lib/services/remediation-default-params.ts` | acknowledge_user empty-params case | VERIFIED | `case 'acknowledge_user':` present |
+| `lib/services/triage-note-service.ts` | generateAndPostAcknowledgment (noteType 18) | VERIFIED | Present, but see Gap #1 (no idempotency) |
+| `lib/services/remediation-service.ts` | acknowledge_user completion → real note post (manual path) | VERIFIED | Idempotent via `alreadyCompleted` check |
+| `components/phishing/classification-card.tsx` | USER_AWARENESS badge + label | VERIFIED | Emerald badge, "Acknowledge user" label |
+| `components/phishing/action-area-card.tsx` | acknowledge_user ActionParamsForm case | VERIFIED | Mirrors `no_action` case |
+| `components/phishing/timeline-card.tsx` | USER_AWARENESS tint + defensive fallback | VERIFIED | 4th VERDICT_TINT key + `||` fallback before `.split()` |
+| `migrations/100_phishing_automation_gate.sql` | opt-in gate table | VERIFIED | Applied to live DB, confirmed via `\d` |
+| `app/api/admin/phishing-automation/route.ts` | admin GET list | VERIFIED | `requireAdmin`, COALESCE defaults |
+| `app/api/admin/phishing-automation/[companyId]/route.ts` | admin PATCH/DELETE | VERIFIED (with WR-05 `any`-cast style nit, non-blocking) | Upsert + revert-to-default present |
+| `.planning/REQUIREMENTS.md` | v3 CLASSDISP-*/AUTOGATE-* entries + Traceability rows | VERIFIED | Present at lines 142-165, 239-244 |
+| `app/admin/phishing-automation/page.tsx` | admin gate page | VERIFIED | 266 lines (min 120), 3 Switches/row, search/filter |
+| `app/admin/page.tsx` | tile linking to gate page | VERIFIED | `href: '/admin/phishing-automation'` at line 250 |
+| `lib/services/phishing-automation-gate.ts` | getCompanyAutomationGate reader | VERIFIED | COALESCE-false reader, 4 passing tests |
+| `lib/services/webhook-service.ts` | gated parse→classify→report chain | VERIFIED (functionally wired) — see Gap #1 for the correctness defect within it | `runGatedPhishingStages` present and called after grouping |
+
+### Key Link Verification
+
+| From | To | Via | Status | Details |
+|------|----|----|--------|---------|
+| classifyCampaign isSimulation branch | USER_AWARENESS verdict | direct assignment | WIRED | `campaign-classifier.ts:462-463` |
+| mapVerdictToActions('USER_AWARENESS') | ['acknowledge_user'] | new switch arm | WIRED | `campaign-classifier.ts:185-186` |
+| remediateApprovedActions acknowledge_user completion | generateAndPostAcknowledgment(campaignId) | post-commit call, alreadyCompleted-gated | WIRED | `remediation-service.ts:224-233` |
+| ClassificationCardData verdict union | VERDICT_VARIANT_CLASS | new keyed entry | WIRED | `classification-card.tsx:34` |
+| action-area-card ActionParamsForm switch | acknowledge_user case | new switch arm | WIRED | `action-area-card.tsx:134-139` |
+| TimelineCard VERDICT_TINT lookup | USER_AWARENESS tint entry | 4th record key + fallback | WIRED | `timeline-card.tsx:22-26,97-98` |
+| GET route | phishing_automation_gate | LEFT JOIN + COALESCE | WIRED | `app/api/admin/phishing-automation/route.ts` |
+| PATCH route | phishing_automation_gate | INSERT ... ON CONFLICT | WIRED | `app/api/admin/phishing-automation/[companyId]/route.ts` |
+| page toggle() | /api/admin/phishing-automation/{companyId} | fetch PATCH | WIRED | `app/admin/phishing-automation/page.tsx` |
+| triggerPhishingDetection (after group) | getCompanyAutomationGate(r.company_id) | gate lookup then conditional stage calls | WIRED | `webhook-service.ts:499,529` |
+| auto_report branch | generateAndPostAcknowledgment(campaignId) | guarded by verdict === 'USER_AWARENESS' | WIRED, but **not idempotency-guarded** | `webhook-service.ts:550-566` — see Gap #1 |
+
+### Data-Flow Trace (Level 4)
+
+Not applicable in the standard "renders dynamic data" sense — this phase is primarily service/API/webhook wiring plus small UI-label additions. The one dynamic-render artifact (`app/admin/phishing-automation/page.tsx`) fetches live company + gate-flag data from `GET /api/admin/phishing-automation`, which itself performs a real `LEFT JOIN`/`COALESCE` query against `companies`/`phishing_automation_gate` — confirmed flowing (no static/empty fallback found).
+
+### Behavioral Spot-Checks
+
+| Behavior | Command | Result | Status |
+|----------|---------|--------|--------|
+| Type check across whole repo | `npx tsc --noEmit --pretty` | No output (clean) | PASS |
+| Phase-23 unit test files | `npx vitest run lib/services/campaign-classifier.test.ts lib/services/remediation-default-params.test.ts lib/services/triage-note-service.test.ts lib/services/remediation-service.test.ts lib/services/phishing-automation-gate.test.ts` | 5 files, 81 tests passed | PASS |
+| Migration 100 applied to live DB | `docker exec pulse-postgres psql -U pulse_user -d pulse_autotask -c '\d phishing_automation_gate'` | Table exists with expected columns/FK | PASS |
+
+Note: `lib/services/webhook-service.ts` has no test file (consistent with CLAUDE.md's stated test-coverage scope), so CR-01's duplicate-note behavior is not caught by any automated test — it was found by direct code trace, matching the code review's finding.
+
+### Probe Execution
+
+Not applicable — this phase has no `scripts/*/tests/probe-*.sh` files and none are referenced in the plans/summaries.
+
+### Requirements Coverage
+
+| Requirement | Source Plan | Description | Status | Evidence |
+|-------------|------------|-------------|--------|----------|
+| CLASSDISP-01 | 23-01 | Classifier assigns USER_AWARENESS to confirmed simulation reports | SATISFIED | `campaign-classifier.ts:458-463` |
+| CLASSDISP-02 | 23-01 | USER_AWARENESS maps to non-destructive acknowledge_user posting noteType 18 | SATISFIED | `campaign-classifier.ts:185-186`, `triage-note-service.ts:236` |
+| CLASSDISP-03 | 23-02 | Review UI surfaces USER_AWARENESS/acknowledge_user distinctly | SATISFIED | classification-card.tsx, action-area-card.tsx, timeline-card.tsx |
+| AUTOGATE-01 | 23-03 | Per-company gate table + admin-gated GET/PATCH/DELETE API, opt-in defaults | SATISFIED | migration 100 + both API routes |
+| AUTOGATE-02 | 23-04 | /admin/phishing-automation page with 3 Switch toggles, backed by AUTOGATE-01 API | SATISFIED | page.tsx + admin index tile; human-verify checkpoint approved live |
+| AUTOGATE-03 | 23-05 | Gated webhook auto-pipeline, acknowledge_user auto-post narrowly scoped to USER_AWARENESS only | **PARTIALLY SATISFIED** | Gating/scoping logic is correct and wired (`webhook-service.ts:522-567`), but the auto-post is not idempotent (CR-01) — a company that opts in will get duplicate customer-visible notes as a campaign accumulates additional reports, which is the normal shape of the data this requirement's own feature exists to handle. This is a functional defect in the requirement's core deliverable, not a cosmetic gap. |
+
+No orphaned requirements — all six IDs from ROADMAP.md's Phase 23 entry are claimed and covered across the 5 plans.
+
+### Anti-Patterns Found
+
+| File | Line | Pattern | Severity | Impact |
+|------|------|---------|----------|--------|
+| `lib/services/webhook-service.ts` | 522-567 (esp. 561) | Missing idempotency guard before customer-visible external side effect | Blocker (see Gap #1 / CR-01) | Duplicate customer-visible Autotask notes on repeat webhook passes for the same campaign |
+| `lib/services/webhook-service.ts` | 550-566 | No `remediation_actions`/`audit_events` row written for the auto-post path (WR-01 in 23-REVIEW.md) | Warning | Action Area UI and TimelineCard have no way to reflect that acknowledge_user was auto-handled; also the root cause enabling CR-01 (nothing to check against) |
+| `lib/services/triage-note-service.ts` | 126-143 (pre-existing, not modified by this phase per file list but flagged in review) | Blast-radius lookup called with empty sender/recipient filters (WR-02) | Warning | Out of this phase's `files_modified` scope for all 5 plans — informational, not a phase-23 introduced regression, but affects a file this phase's manual+auto paths both call into |
+| `app/admin/phishing-automation/page.tsx` | 86-118 | Stale-closure race when toggling two stages on the same row in quick succession (WR-03) | Warning | Rare UI edge case; a fast double-toggle can silently revert one flag; does not block the primary must-have (a single toggle-and-reload cycle works correctly, confirmed via human-verify) |
+| `lib/services/phishing-automation-gate.ts` | 24-58 | Docstring claims "never throws" but the query is unwrapped (WR-04) | Warning | A transient DB error surfaces as an unhandled rejection from this one call site rather than degrading to all-false as documented |
+| `app/api/admin/phishing-automation/[companyId]/route.ts` | 40 | `any` cast on `session.user`, contradicting CLAUDE.md's "no `any`" rule (WR-05) | Warning | Style-convention violation; no functional impact (sibling files in the same phase do this correctly) |
+
+No unresolved `TBD`/`FIXME`/`XXX` debt markers found in the files this phase modified.
+
+### Human Verification Required
+
+None outstanding. Plan 23-04's Task 1 `checkpoint:human-verify` gate was already completed — approved by the human tester against a rebuilt production container (per 23-04-SUMMARY.md), which is accepted as legitimate completed verification per the task brief, not a skipped gate.
+
+### Gaps Summary
+
+Five of the six requirement IDs (CLASSDISP-01/02/03, AUTOGATE-01/02) are fully and correctly implemented, tested, and wired — the classifier disposition, the review-UI surfacing, the gate table/API, and the admin toggle page all hold up under direct code inspection, live DB inspection, and a completed human-verify checkpoint.
+
+AUTOGATE-03 (the webhook auto-pipeline) is wired correctly for gating and action-type scoping, but the phase's own code review already identified — and direct code inspection here confirms is still present and unresolved — a real functional defect: `generateAndPostAcknowledgment()` is called with no idempotency check, so a company with `autoReport` enabled will re-send the customer-visible "thank you for reporting" note to every already-notified ticket each time an additional employee report joins the same phishing campaign. Since phishing campaigns routinely accumulate multiple reports (that's the entire reason `campaign_id` grouping exists), this is not an edge case — it will happen on the very first company that opts into the feature as intended. This directly contradicts the spirit of AUTOGATE-03's own text ("all other verdicts/actions still require manual approval regardless of gate state") in effect, if not in literal action-type scope: the *same* action is repeatedly force-posted without any approval or dedup step, which is the exact "auto-approved" pattern the requirement is trying to avoid duplicating for.
+
+**This looks like an unintentional gap** (the code review already flagged it as CR-01 and proposed a concrete, small fix — check `remediation_actions` for an existing `acknowledge_user` row for the campaign before posting). It does not look like an accepted deviation; no override has been recorded in this or any prior VERIFICATION.md for this phase. Recommend a closure plan implementing the review's suggested fix (idempotency check + persisted remediation_actions/audit_events row for the auto path) before considering Phase 23 complete.
+
+---
+
+_Verified: 2026-07-16T21:20:00Z_
+_Verifier: Claude (gsd-verifier)_