# Project Retrospective

*A living document updated after each milestone. Lessons feed forward into future planning.*

## Milestone: v3.0 — Phishing Triage Automation

**Shipped:** 2026-07-17
**Phases:** 9 (15-23) | **Plans:** 30 | **Sessions:** several, spanning 2026-07-14 through 2026-07-17

### What Was Built
- Full phishing-triage pipeline: detection → EML/MIME parsing → Mimecast blast-radius → deterministic classification → remediation/approval/audit → Autotask triage note → ticket-ID-addressable LiveLink review UI
- A per-Autotask-company automation gate (Phase 23) letting an admin opt companies into a fully automatic parse→classify→acknowledge webhook pipeline while every destructive action stays manual-approval-gated
- Zero-LLM, deterministic classifier with a hardcoded KnowBe4/Breach-Secure-Now simulation-vendor allowlist — avoids both an LLM prompt-injection surface and cry-wolf false THREATs on routine security-awareness tests

### What Worked
- Code review (`/gsd:code-review`) caught two real, non-obvious bugs late in the milestone: a duplicate-campaign grouping bug in Phase 18, and — during this final session — a customer-visible duplicate-note bug in the Phase 23 gap-closure plan itself, plus a second-order defect (missing `actionId` breaking `completedAt` derivation) and a third (no server-side guard on the manual approval path re-triggering the same bug class). All three were fixed and re-verified before shipping, not left for a future session.
- The `--gaps` gap-closure flow (plan → check → execute → re-verify) worked cleanly for a narrowly-scoped, single-defect fix (Phase 23's AUTOGATE-03 idempotency bug) — one new plan, one wave, no disruption to the 5 already-shipped plans in that phase.
- Independent verification via direct Mimecast API calls (not just trusting a ticket's free-text description) caught that one "confirmed Breach Secure Now" report had no actual corroborating inbound message in Mimecast's logs — a real, actionable finding a code-only review would have missed.

### What Was Inefficient
- Phase 22 (Approval UI) shipped all 6 plans and was marked "complete" without ever running `/gsd:verify-work` — this was only discovered at milestone-close time, during the REQUIREMENTS.md traceability check, not during Phase 22 itself. Retroactive verification found the code correct, but this was luck, not process — a phase should never reach "complete" status without a VERIFICATION.md.
- The decision-coverage gate flagged the same class of false-positive three times this milestone (Phase 18 gap closure, initial Phase 23 planning, Phase 23 gap-closure planning): CONTEXT.md decisions substantively implemented but not cited with a literal `D-NN:` prefix under a designated heading. Every instance required manual spot-check-and-override. The gate's designed escape hatch (cite by ID, or mark `[informational]`) isn't being followed by planners in practice — worth either relaxing the gate's matching heuristic or making the citation requirement more prominent in planner instructions.
- Several quick-tasks and REQUIREMENTS.md traceability rows (CLASSIFY-01..06, REVIEW-03) sat with stale "Pending" status for days after their phases actually shipped — pure bookkeeping lag with no functional impact, but it made the milestone-close audit noisier than it needed to be and could mask a real gap next time if not caught.

### Patterns Established
- Idempotency fixes for auto-triggered customer-visible side effects should mirror the existing manual-path pattern in the same file (state write + audit row in one transaction, side-effect call post-commit in its own try/catch) rather than inventing a new shape — this made the Phase 23 gap-closure plan and its review both fast and low-risk.
- When a ticket/ticket-derived claim needs independent confirmation, query the actual upstream system (Mimecast trace/held-message logs) directly rather than trusting a technician's free-text paraphrase of what they saw.

### Key Lessons
1. A phase reaching "6/6 plans complete" is not the same as "verified" — REQUIREMENTS.md traceability status and an actual VERIFICATION.md file should be checked as a gate, not assumed, especially before a milestone close.
2. Code review after a gap-closure fix is not optional busywork — it found real, non-obvious follow-on defects in freshly-written idempotency code twice in this milestone alone.
3. When multiple planning sessions hit the same gate override for the same underlying reason (decision-coverage citation format), that's a signal to fix the gate or the planner convention, not to keep manually overriding it.

### Cost Observations
- Sessions: several across 2026-07-14 → 2026-07-17
- Notable: the final session (gap-closure plan → execute → code-review-catch-and-fix → re-verify → milestone close) ran end-to-end in one sitting, including catching and fixing 2 code-review blockers and running a retroactive Phase 22 verification — the layered gate structure (plan-checker → code-review → verifier) did its job of catching defects before they shipped as "done."

---

## Cross-Milestone Trends

### Process Evolution

| Milestone | Sessions | Phases | Key Change |
|-----------|----------|--------|------------|
| v1.0 | multiple | 11 (1-9.1) | Closed manually, no MILESTONES.md/archive convention yet |
| v2.0 | multiple | 5 (10-14) | Closed manually, same as v1.0 |
| v3.0 | several | 9 (15-23) | First milestone closed via `/gsd:complete-milestone` — established MILESTONES.md, `.planning/milestones/` archive, and this RETROSPECTIVE.md |

### Cumulative Quality

| Milestone | Tests | Coverage | Zero-Dep Additions |
|-----------|-------|----------|-------------------|
| v3.0 | 439/441 repo-wide (2 pre-existing, unrelated failures) | `lib/services/**` per CLAUDE.md's stated scope | 2 (`mailparser`, `linkify-it`) |

### Top Lessons (Verified Across Milestones)

1. Verification gates (plan-checker, code-review, phase-verifier) catch real defects late-stage — don't skip them even under time pressure to close a milestone.
2. Bookkeeping status fields (REQUIREMENTS.md checkboxes, traceability tables) drift from reality if not updated at the moment a phase actually ships — reconcile them explicitly at milestone close rather than assuming they're current.