docs(15-02): add plan 02 summary

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
lorentz 2026-07-15 07:43:43 -04:00
parent 15d0caa20d
commit 92ed5c0e5e

View file

@ -0,0 +1,100 @@
---
phase: 15-data-model-detection-ticket-evidence
plan: 02
subsystem: services
tags: [detection, phishing-triage, evidence, postgres, autotask, sha256]
# Dependency graph
requires:
- phase: 15-01
provides: "reports table (ticket_id UNIQUE FK, content_hash, matched_patterns JSONB, evidence JSONB)"
provides:
- "lib/services/phishing-detector.ts — KNOWN_PHISHING_PATTERNS, matchesPhishingPatterns, computePhishingContentHash, gatherTicketEvidence, detectPhishingTicket"
- "One shared detection entry point (detectPhishingTicket) for both the webhook path and cron sweep in Plan 03"
affects: [15-03-webhook-and-sweep, 16-message-parsing]
# Tech tracking
tech-stack:
added: []
patterns:
- "Case-insensitive substring matcher via .toLowerCase()+.includes() only (no RegExp/eval) — mirrors robotic-classifier.evaluateContains"
- "sha256 content-hash over only the fields that define reprocessing eligibility (title+description), excluding bump-prone fields — mirrors analyzer/preprocessor.computeContentHash"
- "Check-before-write idempotency: SELECT existing content_hash, skip evidence-gathering and write entirely when unchanged — mirrors analyzer/persistence.findExistingAnalysisByContentHash"
key-files:
created: [lib/services/phishing-detector.ts, lib/services/phishing-detector.test.ts]
modified: []
key-decisions:
- "Split Task 1 pure-logic implementation from Task 2 evidence/orchestration into two separate commits (test -> feat -> feat) even though both live in the same file, so the TDD RED/GREEN gate sequence is unambiguous in git history"
- "Autotask client instantiated as a lazy module-level singleton with env-var config, mirroring ticket-reconciliation-service.ts's getClient() pattern, rather than introducing a shared factory (out of scope for this plan)"
- "Idempotency guard compares stored reports.content_hash to the freshly computed hash BEFORE gathering evidence, so an unchanged ticket never re-queries ticket_notes/time_entries/Autotask attachments"
patterns-established:
- "Pattern: detector modules expose pure matching/hashing functions separately from the async DB/API orchestration function, so vitest can cover the pure logic without mocking postgresClient or AutotaskClient"
requirements-completed: [DETECT-01, DETECT-02, EVID-01]
# Metrics
duration: 13min
completed: 2026-07-15
---
# Phase 15 Plan 02: Phishing Detector Summary
**`lib/services/phishing-detector.ts` — a single deterministic detection core matching 8 locked DETECT-01 patterns, sha256 content-hashing for D-04 idempotent reprocessing, and EVID-01 evidence capture (company/notes/time-entries/attachment-metadata) upserted into the Plan 01 `reports` table via `ON CONFLICT (ticket_id)`.**
## Performance
- **Duration:** 13 min
- **Started:** 2026-07-15T11:41:00Z
- **Completed:** 2026-07-15T11:47:59Z
- **Tasks:** 2 completed
- **Files modified:** 2
## Accomplishments
- Implemented `KNOWN_PHISHING_PATTERNS` (the 8 locked DETECT-01 strings verbatim) and `matchesPhishingPatterns` — case-insensitive substring matching (`.toLowerCase()` + `.includes()` only, no `RegExp`/`eval`), returning both a `flagged` boolean and the exact subset of patterns present.
- Implemented `computePhishingContentHash` — sha256 over `{ title, description }` only, stable for identical input, changes on either field, and normalizes `null` description to `''`.
- Wrote and ran a 17-assertion vitest suite (`phishing-detector.test.ts`) covering all 8 patterns individually, the negative case, case-insensitivity, matched[] exactness, and hash stability/change/null-normalization — RED confirmed (module didn't exist) before GREEN implementation.
- Implemented `gatherTicketEvidence` — parameterized `$1` queries against `companies`, `ticket_notes`, and `time_entries`, plus Autotask attachment metadata (`fullPath`/`title`/`contentType` only, never base64 `data`), with the Autotask call wrapped in try/catch so an API failure degrades to an empty attachments array instead of throwing.
- Implemented `detectPhishingTicket` — the shared orchestration entry point: matches, hashes, checks the D-04 idempotency guard (SELECT existing `content_hash`, skip gathering/writing when unchanged), then upserts one `reports` row via `ON CONFLICT (ticket_id) DO UPDATE ... RETURNING id`, binding `requester_contact_id` from `ticket.contact_id` and `created_by_contact_id` from `ticket.created_by_contact_id`.
## Task Commits
Each task was committed atomically, with Task 1 following the full TDD RED/GREEN gate sequence:
1. **Task 1 (RED): add failing tests for pattern matcher + content hash** - `0e7daf9` (test)
2. **Task 1 (GREEN): implement phishing pattern matcher + content hash** - `aabf532` (feat)
3. **Task 2: add evidence capture + detectPhishingTicket orchestration** - `15d0caa` (feat)
**Plan metadata:** (this SUMMARY.md commit)
_Note: Task 1 is TDD — test → feat. No REFACTOR commit was needed; the GREEN implementation was already clean._
## Files Created/Modified
- `lib/services/phishing-detector.ts` - Pure matcher/hash functions (`KNOWN_PHISHING_PATTERNS`, `matchesPhishingPatterns`, `computePhishingContentHash`) plus evidence capture and orchestration (`gatherTicketEvidence`, `detectPhishingTicket`) — the shared detection core for Plan 03's webhook and cron-sweep callers
- `lib/services/phishing-detector.test.ts` - 17 vitest assertions covering all 8 locked patterns individually, negative case, case-insensitivity, matched[] exactness, and content-hash stability/change/null-normalization
## Decisions Made
- Split Task 1's pure-logic commit from Task 2's evidence/orchestration commit even though both extend the same file, so the RED (`test(...)`) → GREEN (`feat(...)`) gate sequence required by the TDD workflow is unambiguous in `git log`, and Task 2's orchestration work is its own reviewable `feat(...)` commit.
- Reused the `ticket-reconciliation-service.ts` lazy-singleton pattern for the AutotaskClient (env-var config, module-level cache) rather than introducing a new factory — no existing `getAutotaskClient()`/`isAutotaskConfigured()` factory was present to reuse, and adding one was out of scope for this plan.
- Idempotency check queries only `id, content_hash` from `reports` (not the full row) and returns immediately on a hash match, before any evidence-gathering queries run — this is what makes the D-04 guarantee ("reprocessing only when content hash changed") cheap for the common case of an unchanged ticket being re-scanned by the cron sweep.
## Deviations from Plan
None - plan executed exactly as written. The plan's own Task 1 instruction ("Do not add DB access in this task — pure functions only") was honored by writing only the pure matcher/hash functions in the first commit, then extending the same file with DB/API-touching code in Task 2's commit, exactly as the plan's two-task structure specifies.
## Issues Encountered
None.
## User Setup Required
None - no external service configuration required. The detector uses the existing `AUTOTASK_*` env vars already configured elsewhere in the codebase (no new credentials introduced).
## Next Phase Readiness
- `detectPhishingTicket(ticket)` is ready to be called from both the Autotask webhook handler and a cron sweep in Plan 03 — same underlying logic, no duplicated matching/hashing/idempotency code between the two callers.
- The reports upsert path is fully wired against the Plan 01 schema (`ON CONFLICT (ticket_id)`, `content_hash`, `matched_patterns` JSONB, `evidence` JSONB) — verified via `npx tsc --noEmit --pretty` and the passing vitest suite; no live DB write was exercised in this plan (that happens when Plan 03 wires a real ticket through the detector against the dev Postgres instance).
- No blockers for Plan 03.
---
*Phase: 15-data-model-detection-ticket-evidence*
*Completed: 2026-07-15*