docs(23): re-verify phase — AUTOGATE-03 gap closed, 18/18 passed
This commit is contained in:
parent
6e8c78b8d2
commit
2e55e5e91d
1 changed files with 73 additions and 89 deletions
|
|
@ -1,151 +1,135 @@
|
|||
---
|
||||
phase: 23-classification-disposition-per-client-automation-gate
|
||||
verified: 2026-07-16T21:20:00Z
|
||||
status: gaps_found
|
||||
score: 17/18 must-haves verified
|
||||
verified: 2026-07-16T23:45:00Z
|
||||
status: passed
|
||||
score: 18/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)"
|
||||
re_verification:
|
||||
previous_status: gaps_found
|
||||
previous_score: 17/18
|
||||
gaps_closed:
|
||||
- "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 (Truth #18 / AUTOGATE-03) — idempotency guard added via autoPostAcknowledgment(), plus two review-found defects in that fix (missing actionId in audit payload; missing manual-path re-approval guard) both closed in a follow-up commit"
|
||||
gaps_remaining: []
|
||||
regressions: []
|
||||
---
|
||||
|
||||
# Phase 23: Classification Disposition + Per-Client Automation Gate Verification Report
|
||||
# Phase 23: Classification Disposition + Per-Client Automation Gate Verification Report (Re-Verification)
|
||||
|
||||
**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.
|
||||
**Phase Goal:** Add a dedicated "User Awareness" verdict for confirmed phishing-simulation-vendor reports, plus an admin UI gate letting an admin choose per-Autotask-company whether the phishing pipeline's parse/classify/report-to-ticket stages run automatically or require manual approval.
|
||||
|
||||
**Verified:** 2026-07-16T21:20:00Z
|
||||
**Status:** gaps_found
|
||||
**Re-verification:** No — initial verification
|
||||
**Verified:** 2026-07-16T23:45:00Z
|
||||
**Status:** passed
|
||||
**Re-verification:** Yes — after gap closure (plan 23-06 + its review-fix commit)
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths
|
||||
|
||||
This re-verification focuses on Truth #18 / AUTOGATE-03 (full 3-level check) and regression-checks Truths #1-17 (existence + diff-based sanity, since their supporting files were untouched by 23-06).
|
||||
|
||||
| # | 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. |
|
||||
| 1-17 | (see original 17 truths — classifier verdict/action mapping, review-UI surfacing, automation-gate table/API/admin-page, gated webhook wiring) | VERIFIED (no regression) | `git diff cc87607..HEAD --stat` shows zero diff for every file backing truths #1-17 (`campaign-classifier.ts`, `remediation-default-params.ts`, `triage-note-service.ts`, `classification-card.tsx`, `action-area-card.tsx`, `timeline-card.tsx`, `migrations/100_phishing_automation_gate.sql`, both admin API routes, `app/admin/phishing-automation/page.tsx`, `phishing-automation-gate.ts`). Full test run of all 5 phase-23 test files (`campaign-classifier.test.ts`, `remediation-default-params.test.ts`, `triage-note-service.test.ts`, `remediation-service.test.ts`, `phishing-automation-gate.test.ts`) passes: 86/86 (was 81/81 at original verification — the +5 are the new `autoPostAcknowledgment` idempotency tests and the CR-02 `approveRemediationActions` guard tests). |
|
||||
| 18 | auto_report auto-posts ONLY the acknowledge_user note for USER_AWARENESS verdicts — no duplicate-note risk on repeat webhook passes for the same campaign — AND the auto-posted row's audit trail correctly feeds `completedAt` derivation, AND the manual approve path cannot re-post an already-auto-acknowledged campaign | **VERIFIED** | Three sub-checks, all confirmed by direct code read + passing tests (see below). |
|
||||
|
||||
**Score:** 17/18 truths verified
|
||||
**Score:** 18/18 truths verified
|
||||
|
||||
#### Truth #18 sub-verification detail
|
||||
|
||||
**(a) Original CR-01 fix — idempotent auto-post, no duplicate notes:**
|
||||
`lib/services/webhook-service.ts:560-562` — the `auto_report` branch now calls `autoPostAcknowledgment(campaignId, 'system:auto_report')` (imported from `./remediation-service`, line 22) instead of the previously-unguarded `generateAndPostAcknowledgment(campaignId)`. `autoPostAcknowledgment` (`remediation-service.ts:355-399`) opens a transaction, takes `SELECT id FROM campaigns WHERE id = $1 FOR UPDATE` (serializes concurrent webhooks for the same campaign), checks `SELECT id FROM remediation_actions WHERE campaign_id = $1 AND action_type = 'acknowledge_user' LIMIT 1` — if a row already exists, returns `{ posted: false }` with **no** insert, **no** audit write, **no** note post. Only on the first pass does it insert a `completed` `remediation_actions` row + write a `remediation_completed` audit_events row (inside the transaction), then post the real note after commit (never inside the transaction; failure caught/logged, never propagated). Confirmed by `remediation-service.test.ts`'s `describe('autoPostAcknowledgment', ...)` block — Test A (first pass posts once), Test B (idempotency — the CR-01 fix — existing row skips insert/audit/note, `posted: false`), Test C (note-post rejection is non-fatal).
|
||||
|
||||
**(b) Review CR-01 fix — audit payload `actionId`:**
|
||||
`remediation-service.ts:371-386` — the INSERT now has `RETURNING id::text AS id`, and the subsequent `writeAuditEvent` payload is `{ actionId: insertRes.rows[0].id, actionType: 'acknowledge_user', auto: true }` (previously omitted `actionId` entirely). `app/api/phishing/campaigns/[id]/route.ts:225-231` (unchanged by this phase — confirmed via `git diff cc87607..HEAD` empty for this file) builds `completedAtByActionId` by reading `event.payload.actionId` from `remediation_completed` audit rows; this now resolves correctly for auto-posted rows exactly as it already did for manually-remediated rows. Test coverage: `remediation-service.test.ts:366-372` asserts the audit call's payload `toMatchObject({ ..., payload: { actionId: 'auto-action-1', actionType: 'acknowledge_user', auto: true } })`.
|
||||
|
||||
**(c) Review CR-02 fix — manual-path re-approval guard:**
|
||||
`remediation-service.ts:112-132` (inside `approveRemediationActions`, previously unguarded) — before validating/inserting each requested action, if `acknowledge_user` is among the requested actions, it runs `SELECT id FROM remediation_actions WHERE campaign_id = $1 AND action_type = 'acknowledge_user' LIMIT 1`; if a row already exists, the loop throws `RemediationValidationError('acknowledge_user has already been posted for this campaign')` for that action **before** any insert happens. This closes the gap the review identified: `approveRemediationActions` → `remediateApprovedActions` → `generateAndPostAcknowledgment` was previously reachable via a direct API call with no server-side defense even after the auto-post path had already sent the note. Test coverage: `remediation-service.test.ts:165-192` — "rejects re-approving acknowledge_user when it has already been posted for the campaign (CR-02 fix)" (asserts zero inserts) and "still allows approving acknowledge_user when no prior post exists for the campaign" (regression guard against over-blocking the legitimate first-approval case).
|
||||
|
||||
### 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 |
|
||||
| `lib/services/remediation-service.ts` | `autoPostAcknowledgment(campaignId, actor)` — idempotent, audit-persisting, `actionId`-carrying auto-post orchestrator | VERIFIED | Lines 324-399; exports `autoPostAcknowledgment` + `AutoPostAcknowledgmentResult`; `RETURNING id::text AS id` + `actionId` in audit payload present |
|
||||
| `lib/services/remediation-service.ts` (`approveRemediationActions`) | Server-side guard against re-approving `acknowledge_user` for an already-auto-posted campaign | VERIFIED | Lines 112-132 |
|
||||
| `lib/services/remediation-service.test.ts` | Tests proving idempotent skip, `actionId` in payload, manual re-approval rejection | VERIFIED | `describe('autoPostAcknowledgment', ...)` (3 tests, lines 353-399) + 2 new tests in `describe('approveRemediationActions', ...)` (lines 165-192); 18/18 tests pass in this file |
|
||||
| `lib/services/webhook-service.ts` | `auto_report` branch calls `autoPostAcknowledgment` instead of the unguarded direct call | VERIFIED | Line 22 (import), lines 560-562 (call); `grep -c generateAndPostAcknowledgment lib/services/webhook-service.ts` returns 0 |
|
||||
| `app/api/phishing/campaigns/[id]/route.ts` | `completedAt` derivation reads `payload.actionId` — now correctly populated for auto-posted rows | VERIFIED (unchanged, now correctly fed) | Lines 221-231; file has zero diff since original verification — confirms the fix is entirely on the write side (`autoPostAcknowledgment`), no read-side change needed |
|
||||
| 14 files backing truths #1-17 | No modification since original verification | VERIFIED — zero diff | Confirmed via individual `git diff cc87607..HEAD -- <file> \| wc -l` = 0 for all 11 distinct artifact files plus the campaigns route |
|
||||
|
||||
### 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 |
|
||||
| `webhook-service.ts` auto_report branch | `autoPostAcknowledgment` | direct call, `verdict === 'USER_AWARENESS'` guard intact | WIRED | `webhook-service.ts:560-562` |
|
||||
| `autoPostAcknowledgment` | `remediation_actions` idempotency check | `SELECT ... action_type = 'acknowledge_user' LIMIT 1` before insert | WIRED | `remediation-service.ts:362-369` |
|
||||
| `autoPostAcknowledgment` | `generateAndPostAcknowledgment` | post-commit call, gated by `inserted === true` | WIRED | `remediation-service.ts:390-396` |
|
||||
| `autoPostAcknowledgment` audit payload | `completedAtByActionId` map | `actionId` field round-trip | WIRED | `remediation-service.ts:382` → `route.ts:225-231` |
|
||||
| `approveRemediationActions` | pre-insert re-approval guard | existence check + throw before per-action insert loop | WIRED | `remediation-service.ts:112-132` |
|
||||
|
||||
### 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).
|
||||
Not applicable in the "renders dynamic data" sense — this closure is service-layer state/audit persistence plus one webhook-branch rewire, no new UI. The one relevant data-flow — `remediation_actions.approved_by` / `audit_events.payload.actionId` written by `autoPostAcknowledgment` flowing into `app/api/phishing/campaigns/[id]/route.ts`'s `completedAtByActionId` map and onward to `RemediationActionSummary.completedAt` — is confirmed flowing (real column reads, no static/empty fallback) since the route file is unchanged and the write-side now emits the shape the route already expected.
|
||||
|
||||
### 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 |
|
||||
| Idempotency + audit + manual-guard unit tests | `npx vitest run lib/services/remediation-service.test.ts` | 18/18 tests passed | PASS |
|
||||
| Full phase-23 test surface (regression check) | `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` | 86/86 tests passed (was 81/81 at original verification) | PASS |
|
||||
| Whole-repo test suite (unrelated-regression check) | `npx vitest run` | 439/441 passed; 2 failures in `lib/services/analyzer/itglue-search.test.ts` | PASS (failures pre-existing, unrelated — see below) |
|
||||
| Commit sequence confirms claimed fix chain | `git log --oneline -10` | `6e8c78b fix(23-06): capture actionId...guard manual re-approval` → `13bf851 fix(23-06): wire auto_report webhook branch` → `c79af9b feat(23-06): add idempotent autoPostAcknowledgment` on top of `a918b72 docs(23): gap-closure plan` on top of `cc87607 docs(23): add phase verification report` | 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.
|
||||
**Note on the 2 whole-repo test failures:** `lib/services/analyzer/itglue-search.test.ts` has 2 failing assertions (`tolerates per-call failures` tests, off-by-one on `result.docs.length`). Confirmed via `git diff cc87607..HEAD -- lib/services/analyzer/itglue-search.ts lib/services/analyzer/itglue-search.test.ts` (zero lines changed) and `git log` on those files (last touched at commit `a0a6e7f`, long before phase 23) that this is a pre-existing, unrelated failure — not introduced by this phase or its gap-closure work. Consistent with the SUMMARY's own disclosure of this issue.
|
||||
|
||||
### Probe Execution
|
||||
|
||||
Not applicable — this phase has no `scripts/*/tests/probe-*.sh` files and none are referenced in the plans/summaries.
|
||||
Not applicable — no `scripts/*/tests/probe-*.sh` files exist and none are referenced in the plans/summaries for this phase or its gap-closure plan.
|
||||
|
||||
### 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. |
|
||||
| CLASSDISP-01 | 23-01 | Classifier assigns USER_AWARENESS to confirmed simulation reports | SATISFIED (no regression) | Unchanged since original verification |
|
||||
| CLASSDISP-02 | 23-01 | USER_AWARENESS maps to non-destructive acknowledge_user posting noteType 18 | SATISFIED (no regression) | Unchanged since original verification |
|
||||
| CLASSDISP-03 | 23-02 | Review UI surfaces USER_AWARENESS/acknowledge_user distinctly | SATISFIED (no regression) | Unchanged since original verification |
|
||||
| AUTOGATE-01 | 23-03 | Per-company gate table + admin-gated GET/PATCH/DELETE API, opt-in defaults | SATISFIED (no regression) | Unchanged since original verification |
|
||||
| AUTOGATE-02 | 23-04 | /admin/phishing-automation page with 3 Switch toggles, backed by AUTOGATE-01 API | SATISFIED (no regression) | Unchanged since original verification |
|
||||
| AUTOGATE-03 | 23-05, 23-06 | Gated webhook auto-pipeline, acknowledge_user auto-post narrowly scoped to USER_AWARENESS only, idempotent, audit-consistent, and defended against duplicate manual re-approval | **SATISFIED** (upgraded from PARTIALLY SATISFIED) | `webhook-service.ts:560-562` + `remediation-service.ts:112-132,324-399` + 5 new passing tests |
|
||||
|
||||
No orphaned requirements — all six IDs from ROADMAP.md's Phase 23 entry are claimed and covered across the 5 plans.
|
||||
No orphaned requirements — all six IDs from ROADMAP.md's Phase 23 entry are claimed and covered. `.planning/REQUIREMENTS.md`'s Traceability table still lists all six as "Pending" (lines 239-244) — this is the same pre-existing housekeeping-only discrepancy noted (non-blocking) in the original verification's Truth #14; it was not addressed by plan 23-06 (out of that plan's scope) and does not affect this re-verification's functional findings.
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
None in the files touched by this gap-closure (`remediation-service.ts`, `remediation-service.test.ts`, `webhook-service.ts`) — no `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER` markers found.
|
||||
|
||||
One warning-level item from `23-06-REVIEW.md` remains open and was not required to be closed by this re-verification's scope (it was categorized Warning, not Critical, and is not part of Truth #18's observable contract):
|
||||
|
||||
| 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) |
|
||||
| `lib/services/remediation-service.ts` | 355-360 (`autoPostAcknowledgment`) | No explicit "campaign not found" guard (WR-01 in `23-06-REVIEW.md`), unlike `markCampaignFalsePositive`'s equivalent check | Warning (non-blocking) | A bad/stale `campaignId` would fail via an opaque Postgres FK-violation on the `INSERT INTO remediation_actions` rather than a typed `RemediationValidationError`; caught by `webhook-service.ts`'s surrounding try/catch so it does not crash the request or break Truth #18's idempotency contract — purely a diagnostics/consistency nit |
|
||||
|
||||
No unresolved `TBD`/`FIXME`/`XXX` debt markers found in the files this phase modified.
|
||||
The other review Info-level item (`IN-01` — `posted: true` even when the note itself failed to send) is confirmed still present but is explicitly documented in the review as intentional design mirroring the already-VERIFIED manual path's identical trade-off — not a defect.
|
||||
|
||||
No unresolved `TBD`/`FIXME`/`XXX` debt markers found in any file touched by this phase or its gap closure.
|
||||
|
||||
### 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.
|
||||
None. This gap-closure plan introduced no new UI surface; the one prior human-verify checkpoint (Truth #15, plan 23-04's admin toggle page) was already completed and is unaffected by this closure (its backing file has zero diff since the original verification).
|
||||
|
||||
### 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.
|
||||
None remaining. The single gap from the original verification (Truth #18 / AUTOGATE-03 — missing idempotency guard on the webhook auto-post path, CR-01) is closed: `autoPostAcknowledgment()` now makes the auto-post idempotent per campaign via a `campaigns` row lock + `remediation_actions` existence check, persisting both a `remediation_actions` row and a `remediation_completed` audit_events row on first post only.
|
||||
|
||||
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.
|
||||
The subsequent code-review pass on that closure (`23-06-REVIEW.md`) found two further real defects in the fix itself — both confirmed fixed by direct code inspection in this re-verification:
|
||||
1. The audit payload now includes `actionId` (via `RETURNING id::text AS id` on the insert), correctly feeding `completedAt` derivation in the campaign-detail API for auto-posted rows.
|
||||
2. `approveRemediationActions` (the manual approve path) now has a server-side guard rejecting re-approval of `acknowledge_user` when a prior post (auto or manual) already exists for the campaign — closing the direct-API-call bypass of the client-side-only UI gate.
|
||||
|
||||
**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.
|
||||
All 5 new/modified tests pass, `npx tsc --noEmit --pretty` is clean, and no file backing any of the other 17 previously-VERIFIED truths shows any diff since the original verification (`git diff cc87607..HEAD --stat` confirms the only production-code changes are `lib/services/remediation-service.ts` and `lib/services/webhook-service.ts`, exactly matching plan 23-06's declared `files_modified` plus the review-fix commit on the same two files).
|
||||
|
||||
Phase 23 is now fully verified: 18/18 must-haves, 6/6 requirement IDs SATISFIED, no regressions, no outstanding human-verification items.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-07-16T21:20:00Z_
|
||||
_Verified: 2026-07-16T23:45:00Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue