chore: merge executor worktree (worktree-agent-af09d50545da22d95)

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

@@ -0,0 +1,110 @@
+---
+phase: 18-campaign-grouping-phishing-analysis-api
+plan: 04
+subsystem: api
+tags: [postgres, vitest, tdd, campaign-grouping, phishing]
+
+# Dependency graph
+requires:
+  - phase: 18-01
+    provides: groupReportIntoCampaign tiered matching (Tier 1/2/3), campaigns find-or-create core
+  - phase: 18-03
+    provides: GET /api/phishing/campaigns list route
+provides:
+  - Idempotent re-grouping — re-running /analyze on an already-grouped report no longer double-increments campaigns.report_count
+  - Regression test proving the sibling-re-match no-op path
+  - Clamped [0, 200] limit query param on GET /api/phishing/campaigns (explicit limit=0 honored, negative limit no longer 500s)
+affects: [19-classification, 20-remediation-approval-audit, 21-autotask-triage-note]
+
+# Tech tracking
+tech-stack:
+  added: []
+  patterns:
+    - "Own-row current-state guard before mutating an aggregate: fetch the row's own current FK (campaign_id) alongside its match candidates, and short-circuit as a no-op when a tiered/derived match resolves back to the row's own current state — prevents unbounded self-reinforcing increments under repeat invocation."
+    - "Parse-then-clamp for untrusted numeric query params: parse once with Number.isFinite (not `|| default`, which swallows explicit 0), then Math.min(Math.max(parsed, floor), ceiling)."
+
+key-files:
+  created: []
+  modified:
+    - lib/services/campaign-grouping-service.ts
+    - lib/services/campaign-grouping-service.test.ts
+    - app/api/phishing/campaigns/route.ts
+
+key-decisions:
+  - "Guard placed inside the match branch (matchCampaignId === ownReport.campaign_id) rather than adding a new pre-check query — reuses the ownReport row already fetched, no extra round trip"
+  - "Did not implement campaign-move / origin-count-decrement — out of scope per VERIFICATION.md, that code path doesn't exist today"
+
+requirements-completed: [CAMP-02, CAMP-03]
+
+duration: 2min
+completed: 2026-07-16
+---
+
+# Phase 18 Plan 04: Campaign re-match idempotency + limit param clamp Summary
+
+**Fixed unbounded `campaigns.report_count` double-increment on repeat `/analyze` calls by short-circuiting when a tiered match resolves to the report's own current campaign, plus clamped the `limit` query param on `GET /api/phishing/campaigns` to [0, 200].**
+
+## Performance
+
+- **Duration:** 2 min
+- **Started:** 2026-07-16T02:23:52Z
+- **Completed:** 2026-07-16T02:25:43Z
+- **Tasks:** 2 completed
+- **Files modified:** 3
+
+## Accomplishments
+- Closed the CAMP-02 gap (CR-01 in 18-REVIEW.md): re-running `POST /api/phishing/tickets/{id}/analyze` on a report already linked to a multi-report campaign no longer inflates that campaign's `report_count`.
+- Added a regression test (`campaign-grouping-service.test.ts`) that fails without the fix and proves zero `UPDATE campaigns` / `INSERT INTO campaigns` / `UPDATE reports SET campaign_id` calls on a same-campaign sibling re-match.
+- Closed the WR-02 finding: `GET /api/phishing/campaigns?limit=-5` no longer raises an unhandled Postgres 500 ("LIMIT must not be negative"), and `limit=0` is now honored instead of silently replaced by the default of 50.
+- Verified no regression: all 16 pre-existing grouping tests plus the new test are green (17/17); `npx tsc --noEmit --pretty` exits 0.
+
+## Task Commits
+
+Each task was committed atomically (Task 1 followed RED → GREEN TDD flow):
+
+1. **Task 1 (RED): add failing regression test** - `afd7af7` (test)
+2. **Task 1 (GREEN): short-circuit own-campaign re-match** - `9ca2ccf` (fix)
+3. **Task 2: clamp campaigns list limit param (WR-02)** - `abe3d4b` (fix)
+
+**Plan metadata:** committed with this SUMMARY.md (see final commit)
+
+## Files Created/Modified
+- `lib/services/campaign-grouping-service.ts` - `OwnReportRow` now includes `campaign_id: string | null`; the own-report SELECT fetches `campaign_id::text AS campaign_id` (appended after `created_at` to preserve the test-mock routing substring); the match branch short-circuits to a no-op (`created: false`, no UPDATEs) when `matchCampaignId === ownReport.campaign_id`.
+- `lib/services/campaign-grouping-service.test.ts` - New test: "re-analyzing a report already linked to a campaign that a sibling report re-matches does not increment report_count a second time" — stages `ownReport.campaign_id: 'campaign-1'` with a Tier 3 sibling match on the same campaign, asserts zero mutating calls.
+- `app/api/phishing/campaigns/route.ts` - `limit` parsing changed from `Math.min(parseInt(...) || 50, 200)` to parse-once-then-clamp: `Math.min(Math.max(Number.isFinite(rawLimit) ? rawLimit : 50, 0), 200)`. `offset`, `status` filter, auth gate, and response shape unchanged.
+
+## Decisions Made
+- Reused the already-fetched `ownReport` row for the no-op guard instead of an extra query — the row is already in scope inside the transaction, so no additional round trip is needed.
+- Left the campaign-move / origin-decrement path unimplemented, per VERIFICATION.md's explicit scoping — that behavior doesn't exist anywhere in the codebase today and was called out as conditionally out of scope ("if ever supported").
+
+## Deviations from Plan
+
+None - plan executed exactly as written. Both tasks matched their specified `<action>` blocks precisely (including the exact SQL column-list append point and the `Number.isFinite` clamp shape).
+
+## TDD Gate Compliance
+
+Task 1 was `tdd="true"`. Gate sequence verified in git log:
+1. RED: `afd7af7 test(18-04): add failing regression test for campaign report_count double-increment` — test run confirmed failure before the fix (1 failed / 16 passed).
+2. GREEN: `9ca2ccf fix(18-04): short-circuit own-campaign re-match to stop report_count double-increment` — test run confirmed all 17 tests pass after the fix.
+3. No REFACTOR commit needed — implementation was minimal and required no cleanup pass.
+
+Note: the GREEN commit uses `fix(...)` rather than `feat(...)` since this is a bug fix (per the project's commit-type convention: `fix` for bug corrections), not new feature addition. The RED→GREEN sequencing (test commit before implementation commit) is the gate requirement and is satisfied.
+
+## Issues Encountered
+None.
+
+## User Setup Required
+
+None - no external service configuration required.
+
+## Next Phase Readiness
+- CAMP-02 and CAMP-03 requirements are now fully closed; `campaigns.report_count` is accurate and idempotent under repeat `/analyze` calls, and the read API (`GET /api/phishing/campaigns`) safely handles malformed pagination params.
+- No blockers for Phase 19 (Classification), which consumes campaign data as an input.
+
+---
+*Phase: 18-campaign-grouping-phishing-analysis-api*
+*Completed: 2026-07-16*
+
+## Self-Check: PASSED
+
+All created/modified files verified present; all commit hashes (afd7af7, 9ca2ccf, abe3d4b) verified in git log.

app/api/phishing/campaigns/route.ts

@@ -28,7 +28,8 @@ export async function GET(request: NextRequest) {
 
   try {
     const url = request.nextUrl;
-    const limit = Math.min(parseInt(url.searchParams.get('limit') ?? '50', 10) || 50, 200);
+    const rawLimit = parseInt(url.searchParams.get('limit') ?? '50', 10);
+    const limit = Math.min(Math.max(Number.isFinite(rawLimit) ? rawLimit : 50, 0), 200);
     const offset = Math.max(parseInt(url.searchParams.get('offset') ?? '0', 10) || 0, 0);
     const status = url.searchParams.get('status');
 

lib/services/campaign-grouping-service.test.ts

@@ -333,4 +333,31 @@ describe('groupReportIntoCampaign', () => {
     // a second time for this report re-run.
     expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
   });
+
+  it('re-analyzing a report already linked to a campaign that a sibling report re-matches does not increment report_count a second time', async () => {
+    // Own report is already linked to campaign-1. A SIBLING report (not this
+    // report's own excluded row) also belongs to campaign-1 and matches via
+    // Tier 3 (sender + subject + client). This is the CAMP-02/CR-01 gap:
+    // tier queries only exclude the report's OWN row, not siblings already
+    // in the same campaign, so a naive re-match would double-increment.
+    stage({
+      ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-1' }],
+      ownMessage: [],
+      tier3: [{ campaign_id: 'campaign-1', title: 'Invoice Alert' }],
+    });
+
+    const result = await groupReportIntoCampaign('report-self');
+
+    expect(result).toEqual({
+      campaignId: 'campaign-1',
+      groupMethod: 'sender_subject_client',
+      created: false,
+    });
+
+    // Critical assertions: no double-increment, no phantom new campaign, no
+    // redundant re-link.
+    expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
+    expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0);
+    expect(callsContaining('UPDATE reports SET campaign_id')).toHaveLength(0);
+  });
 });

lib/services/campaign-grouping-service.ts

@@ -66,6 +66,7 @@ interface OwnReportRow {
   requester_contact_id: number | null;
   company_id: number | null;
   created_at: string;
+  campaign_id: string | null;
 }
 
 interface OwnMessageRow {
@@ -154,7 +155,7 @@ export async function groupReportIntoCampaign(
 
     return await postgresClient.transaction(async (client) => {
       const ownReportRes = await client.query<OwnReportRow>(
-        `SELECT title, requester_contact_id, company_id, created_at
+        `SELECT title, requester_contact_id, company_id, created_at, campaign_id::text AS campaign_id
            FROM reports
           WHERE id = $1`,
         [reportId]
@@ -321,6 +322,16 @@ export async function groupReportIntoCampaign(
       // Find-or-create against `campaigns` (CAMP-02).
       // ---------------------------------------------------------------------
       if (matchCampaignId && matchGroupMethod) {
+        // CR-01/CAMP-02 no-op guard: tier queries only exclude the report's
+        // OWN row (`r.id != $x`), not its siblings already in the same
+        // campaign. Since `/analyze` re-runs grouping unconditionally
+        // (D-08), a report already linked to campaign X can re-match X via
+        // a sibling row on every repeat call, inflating `report_count`
+        // without bound. If the match resolves to the report's own current
+        // campaign, it's already correctly linked — skip both UPDATEs.
+        if (matchCampaignId === ownReport.campaign_id) {
+          return { campaignId: matchCampaignId, groupMethod: matchGroupMethod, created: false };
+        }
         await client.query(
           `UPDATE campaigns
               SET report_count = report_count + 1, last_seen_at = NOW(), updated_at = NOW()