docs(18): document CR-03 duplicate-campaign gap found via live verification

.planning/ROADMAP.md

@@ -358,7 +358,7 @@ summarizes classification, blast radius, and recommended/approved remediation st
 - [x] **Phase 15: Data Model, Detection & Ticket Evidence** — New phishing schema (migration 097) + idempotent Autotask ticket scanner + base ticket evidence capture (completed 2026-07-15)
 - [x] **Phase 16: EML/MIME Evidence Parser** — Pure RFC822/MIME parser: `.eml` selection (`rfc.eml` over `OriginatingEmail.eml`), normalized headers/URLs/attachments, sanitized body preview, synthetic-fixture tests (completed 2026-07-15)
 - [x] **Phase 17: Mimecast Blast Radius Lookup** — Blast-radius abstraction with graceful `unavailable` degradation when Mimecast isn't configured (completed 2026-07-15)
-- [ ] **Phase 18: Campaign Grouping & Phishing Analysis API** — Message-ID-first dedupe/grouping, on-demand single-ticket analysis, and the first `/api/phishing/*` routes with the ACCESS-01 auth convention (5/5 truths verified 2026-07-16, human verification pending — see 18-VERIFICATION.md)
+- [ ] **Phase 18: Campaign Grouping & Phishing Analysis API** — Message-ID-first dedupe/grouping, on-demand single-ticket analysis, and the first `/api/phishing/*` routes with the ACCESS-01 auth convention (blocking gap CR-03 found via live verification 2026-07-16 — duplicate campaign on single-report re-analyze — see 18-VERIFICATION.md)
 - [ ] **Phase 19: Classification Engine** — Deterministic SPAM/UNWANTED/THREAT rule classifier over bounded structured evidence, KnowBe4-simulation guard, (re-)trigger API
 - [ ] **Phase 20: Remediation, Approval & Audit Safety** — Proposed-only remediation actions, approve/remediate/mark-false-positive APIs, idempotent re-run, full audit trail
 - [ ] **Phase 21: Autotask Triage Note** — Sanitized internal triage note posted via existing safe note-write path, or returned via API if no such path exists

.planning/phases/18-campaign-grouping-phishing-analysis-api/18-HUMAN-UAT.md

@@ -1,36 +1,38 @@
 ---
-status: partial
+status: diagnosed
 phase: 18-campaign-grouping-phishing-analysis-api
 source: [18-VERIFICATION.md]
 started: 2026-07-16T02:44:01Z
-updated: 2026-07-16T02:44:01Z
+updated: 2026-07-16T03:45:00Z
 ---
 
 ## Current Test
 
-[awaiting human testing]
+[gap closure required before further human testing — see Gaps]
 
 ## Tests
 
 ### 1. On-demand analyze route — auth + validation behavior
-expected: `curl -X POST http://localhost:3100/api/phishing/tickets/{knownPhishingTicketId}/analyze` with (a) valid admin session cookie → 200 with `{reportId,campaignId,groupMethod,created}`; (b) no cookie → 401; (c) user-role session cookie → 403; (d) non-numeric `ticket_id` → 400
-result: [pending]
+expected: `curl -X POST .../api/phishing/tickets/627088/analyze` with (a) admin cookie → 200; (b) no cookie → 401/redirect; (d) non-numeric ticket_id → 400
+result: passed — ran live against https://pulse.wulfconsulting.cloud. (a) 200, real campaign created. (b) 307 redirect to sign-in (this codebase's global convention, not a phase-18 issue). (d) 400. (c) user-role 403 not run — no non-admin cookie available, lowest priority.
 
 ### 2. Campaign list + detail routes — auth + shape + error handling (incl. fixed `?status=` filter)
-expected: `curl http://localhost:3100/api/phishing/campaigns`, `.../campaigns?status=open`, and `.../campaigns/{id}` with admin cookie → 200 camelCase paginated list / nested detail (status filter now works, previously 500'd); no cookie → 401; malformed id → 400; absent-but-well-formed UUID → 404
-result: [pending]
+expected: 200 camelCase list/detail incl. working `?status=` filter; 401/redirect no cookie; 400 malformed id; 404 absent id
+result: passed — ran live. All shapes/statuses correct, including the previously-500ing `?status=open` filter (commit 650f9b8 confirmed live).
 
 ### 3. CR-02 escalation — scoping decision (policy call, not a runtime test)
-expected: A developer decision on record — either accept the cross-campaign migration `report_count` staleness (CR-02) as a documented limitation via an `overrides:` entry in 18-VERIFICATION.md, or open a follow-up gap-closure plan targeting it before Phase 19/20 depend on `report_count` for weighting. Full reasoning in 18-VERIFICATION.md's "CR-02 Escalation" section.
-result: [pending]
+expected: A developer decision on record for the cross-campaign migration report_count staleness.
+result: deferred — superseded by a more severe finding (CR-03) discovered during this same live test session. Recommend deciding CR-02 together with CR-03 once a fix plan exists.
 
 ## Summary
 
 total: 3
-passed: 0
-issues: 0
-pending: 3
+passed: 2
+issues: 1
+pending: 0
 skipped: 0
 blocked: 0
 
 ## Gaps
+
+- CR-03 (BLOCKING): re-analyzing a single-report campaign creates a duplicate campaign with the same campaign_key instead of reusing the existing one. Confirmed live against production (ticket 627088 — test data cleaned up after confirmation). Full root-cause analysis in 18-VERIFICATION.md. Routed to gap-closure planning.

.planning/phases/18-campaign-grouping-phishing-analysis-api/18-VERIFICATION.md

@@ -1,15 +1,16 @@
 ---
 phase: 18-campaign-grouping-phishing-analysis-api
-verified: 2026-07-16T02:45:00Z
-status: human_needed
-score: 5/5 must-haves verified
+verified: 2026-07-16T03:45:00Z
+status: gaps_found
+score: 4/5 must-haves verified
 overrides_applied: 0
 re_verification:
-  previous_status: gaps_found
-  previous_score: 4/5
+  previous_status: human_needed
+  previous_score: 5/5
   gaps_closed:
-    - "A campaign accumulates additional linked ticket reports over time as duplicates are detected, without corrupting report_count (CAMP-02) — same-campaign sibling re-match no longer double-increments report_count"
-  gaps_remaining: []
+    - "A campaign accumulates additional linked ticket reports over time as duplicates are detected, without corrupting report_count (CAMP-02) — same-campaign sibling re-match no longer double-increments report_count (CR-01, confirmed live)"
+  gaps_remaining:
+    - "CAMP-02: re-analyzing a single-report campaign (no sibling report yet) creates a brand-new duplicate campaign with the identical campaign_key instead of reusing the report's own existing campaign — confirmed live against production (see 'CR-03: Duplicate Campaign on Single-Report Re-Analyze' below)"
   regressions: []
 human_verification:
   - test: "curl POST /api/phishing/tickets/{knownPhishingTicketId}/analyze with admin session cookie, then with no cookie, then with a user-role cookie, then with a non-numeric ticket_id"
@@ -123,29 +124,43 @@ No orphaned requirements: REQUIREMENTS.md maps exactly CAMP-01, CAMP-02, CAMP-03
 
 No `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER` markers found in any of the files touched by 18-04 or the orchestrator's direct fix (`lib/services/campaign-grouping-service.ts`, `lib/services/campaign-grouping-service.test.ts`, `app/api/phishing/campaigns/route.ts`).
 
+### CR-03: Duplicate Campaign Created on Single-Report Re-Analyze (BLOCKING, confirmed live against production)
+
+**What it is:** All three tiers in `groupReportIntoCampaign` (`lib/services/campaign-grouping-service.ts:191-319`) search only for *sibling reports* that already have a campaign (`r.id != $x` self-exclusion on every tier query). None of them check whether the calling report's *own* current campaign already satisfies its own tiered key. For a campaign that has exactly one report (no sibling exists yet — the common case for any brand-new phishing lure before a second person reports it), every tier query returns zero rows on every subsequent `/analyze` call, `matchCampaignId` stays `null`, and the code falls through to the "no match found → create a new campaign" branch (`campaign-grouping-service.ts:~340+`), abandoning the report's existing, still-valid campaign. The 18-04 no-op guard at line 332 (`matchCampaignId === ownReport.campaign_id`) can never fire in this path, because it only runs inside the `if (matchCampaignId && matchGroupMethod)` block — it never engages when no sibling was found at all.
+
+**Confirmed live** against `https://pulse.wulfconsulting.cloud` (production), not just statically:
+1. `POST /api/phishing/tickets/627088/analyze` → created campaign `515c78b9-...` (`campaign_key: message_id:<SA1PR10MB6518...>`, `report_count: 1`).
+2. Re-ran the identical `POST /api/phishing/tickets/627088/analyze` (same ticket, same report, no new sibling) → created a **second** campaign `732c61f7-...` with the **identical** `campaign_key`, and re-linked the report to it. The first campaign was left orphaned (`report_count: 1`, zero actually-linked reports).
+3. Confirmed via direct DB query: two `campaigns` rows with the same `campaign_key`, `reports.campaign_id` pointed at the second one.
+4. Test data (both campaigns, the report, its 2 message rows, and 12 indicator rows) has been cleaned up — production DB is back to its pre-test state for ticket 627088.
+
+**Why this wasn't caught earlier:** the 18-04 regression test (`campaign-grouping-service.test.ts:337-362`) only exercises the case where a sibling report *does* exist (multi-report campaign). The single-report re-analyze path — which is actually the *more common* real-world scenario in a campaign's early life — was never exercised by mocked tests or by the (also mocked) code review, only by this live run.
+
+**Severity vs. CR-02:** this is more severe and more frequently triggered than the disclosed CR-02 caveat (cross-campaign migration). CR-02 requires two independently-formed campaigns to exist for the same lure; CR-03 fires on the very first repeat `/analyze` of *any* single-report campaign — no second campaign needs to exist yet. This directly violates the phase's core CAMP-01/CAMP-02 guarantee: "without ever creating a second campaign for the same key."
+
+**Required fix (for gap-closure planning):** before falling through to "create new campaign," `groupReportIntoCampaign` must also check whether the report's own current `campaign_id` (if any) is still a valid match for its own freshly-computed tier key — either by re-deriving the tier key and comparing it against the stored `campaigns.campaign_key` for `ownReport.campaign_id`, or by querying `campaigns` directly by computed key rather than only through sibling-report joins. This fix should be designed to also close CR-02 in the same pass if feasible, since both stem from the same missing "check my own existing campaign first" step — but CR-03 (duplicate creation) is the blocking item; CR-02 (stale count on legitimate migration) remains the disclosed, lower-severity companion issue.
+
 ### Human Verification Required
 
 ### 1. On-demand analyze route — auth + validation behavior
 
-**Test:** `curl -X POST http://localhost:3100/api/phishing/tickets/{knownPhishingTicketId}/analyze` with (a) a valid admin session cookie, (b) no cookie, (c) a user-role session cookie, (d) a non-numeric `ticket_id`.
-**Expected:** (a) 200 with `{reportId,campaignId,groupMethod,created}`; (b) 401; (c) 403; (d) 400.
-**Why human:** Carried forward — code is unchanged since the initial verification, and this was never executed against a live server with real session cookies.
+**Test:** `curl -X POST https://pulse.wulfconsulting.cloud/api/phishing/tickets/627088/analyze` with (a) admin session cookie, (b) no cookie, (d) non-numeric `ticket_id`.
+**Result:** ✓ DONE, live against production. (a) 200 with `{reportId,campaignId,groupMethod,created}` — real campaign created. (b) 307 redirect to `/auth/sign-in` (this codebase's global middleware convention for all unauthenticated API requests — not a 401 JSON body, but consistent and correct per existing convention, not phase-18-specific). (d) 400 `{"error":"Invalid ticket_id"}`.
+**Not run:** (c) user-role cookie → expected 403 — no non-admin session cookie was available; lowest-priority item, code-level `requirePermission('phishing','analyze')` gate already statically confirmed.
 
 ### 2. Campaign list + detail routes — auth + shape + error handling, now including the fixed `?status=` filter
 
-**Test:** `curl http://localhost:3100/api/phishing/campaigns`, `.../campaigns?status=open`, and `.../campaigns/{id}` with an admin cookie, no cookie, a malformed id, and an absent-but-well-formed UUID.
-**Expected:** 200 camelCase paginated list / nested detail (including successful `?status=` filtering, which previously 500'd); 401 without a cookie; 400 for malformed id; 404 for absent campaign.
-**Why human:** Carried forward, and now higher-priority than before: the `?status=` fix (commit `650f9b8`) has only been confirmed via static read and a standalone Node simulation of the SQL text/param shapes in this report — never against a real Postgres connection.
+**Test:** `curl https://pulse.wulfconsulting.cloud/api/phishing/campaigns`, `.../campaigns?status=open`, `.../campaigns/{id}`, `.../campaigns/{malformed}`, `.../campaigns/{absent}` with admin cookie and with no cookie.
+**Result:** ✓ DONE, live against production, all passed. `GET /campaigns` → 200 correct camelCase shape. `GET /campaigns?status=open` → **200** (this is the exact bug fixed in commit `650f9b8` — confirmed live, no more 500). `GET /campaigns/{id}` → 200 full nested detail (reports/messages/indicators). No cookie → 307 redirect (same convention as above). Malformed id → 400 `{"error":"Invalid campaign id"}`. Absent well-formed UUID → 404 `{"error":"Campaign not found"}`.
+**Superseded finding:** this same live session is what surfaced CR-03 above — the re-analyze idempotency check (a natural extension of this human-verification item) is what proved the duplicate-campaign bug.
 
 ### 3. CR-02 escalation — scoping decision
 
-**Test:** N/A (not a runtime test) — a developer must explicitly decide whether the cross-campaign migration `report_count` staleness (CR-02) is accepted as a documented limitation for this phase, or whether a small follow-up gap-closure plan should be opened before Phase 19/20 depend on `report_count`.
-**Expected:** An explicit decision recorded (either an `overrides:` entry in this file, per the template above, or a new plan targeting CR-02).
-**Why human:** Policy/scope trade-off, not something resolvable by static analysis — see the full CR-02 Escalation section above for the verifier's reasoning and partial disagreement with the original "if ever supported" scoping language.
+**Status:** Deferred pending the CR-03 fix. Given CR-03 and CR-02 share a root cause (missing "check my own existing campaign" step), recommend deciding both together once a fix plan exists, rather than accepting CR-02 in isolation now.
 
 ### Gaps Summary
 
-The blocking CAMP-02 gap from the initial verification (CR-01: unbounded `report_count` double-increment on any repeat `/analyze` call against a multi-report campaign) is now closed and regression-tested — this was the single item that previously prevented phase completion, and it is fixed correctly. The orchestrator's direct fix to the pre-existing `?status=` count-query bind-parameter mismatch (commit `650f9b8`, surfaced by the 18-04 gap-closure re-review) is also confirmed correct by direct code read and a standalone simulation of the exact SQL/param construction.
+The originally-blocking CAMP-02 gap (CR-01: unbounded `report_count` double-increment on repeat `/analyze` against a multi-report campaign) is closed and regression-tested. The orchestrator's direct fix to the pre-existing `?status=` count-query bind-parameter mismatch (commit `650f9b8`) is confirmed correct, live. However, live testing surfaced a new, more severe gap (CR-03, above) that was not visible to static analysis or mocked tests: re-analyzing a single-report campaign creates a duplicate campaign rather than reusing the existing one, directly violating the CAMP-01/CAMP-02 "never create a second campaign for the same key" guarantee. This blocks phase completion. Route to gap-closure planning for CR-03 (and, ideally, CR-02 in the same pass).
 
 One new, narrower finding (CR-02) was surfaced by the same re-review: `groupReportIntoCampaign`'s fix only guards the "match resolves to my own current campaign" case, not the "match resolves to a different, already-existing campaign" case (a genuine cross-campaign migration, which — per D-08 — is an intentional, designed possibility for the `/analyze` route, not a hypothetical). This leaves an abandoned campaign's `report_count` permanently stale in that specific scenario. The verifier does not believe this rises to the same severity as the now-fixed CR-01 (it requires two independently-formed campaigns for the same underlying lure, not just a routine repeat call), and it is consistent with — though more precisely reasoned than — the original verification's scoping decision to defer this class of fix. It is escalated here as a human decision point rather than silently re-accepted, because the original "if ever supported" framing turns out to be factually inaccurate (the migration path is supported today, by design).