diff --git a/.planning/RETROSPECTIVE.md b/.planning/RETROSPECTIVE.md new file mode 100644 index 0000000..abd3ae8 --- /dev/null +++ b/.planning/RETROSPECTIVE.md @@ -0,0 +1,59 @@ +# 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.