docs(260716-n46): add plan SUMMARY and deferred-items log

Documents the getBlastRadius per-tenant injection/swallowed-error fixes, the campaign detail route's date-window clamp + tenant resolution, the in-scope test-hygiene deviation, and the pre-existing out-of-scope itglue-search.test.ts failure.
2026-07-16 16:48:29 -04:00 · 2026-07-16 16:48:29 -04:00 · 1e384d5c9e
commit 1e384d5c9e
parent 9951e53832
2 changed files with 120 additions and 0 deletions
--- a/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md
+++ b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md
@ -0,0 +1,115 @@
+---
+phase: quick-260716-n46
+plan: 01
+subsystem: api
+tags: [mimecast, blast-radius, phishing, multi-tenant, ticket-699308]
+
+requires:
+  - phase: 17-mimecast-blast-radius-lookup
+    provides: getBlastRadius() fan-out + getMimecastClientForTenant() factory
+  - phase: 22-approval-ui-livelink-addressable-campaign-review-and-approve
+    provides: app/api/phishing/campaigns/[id]/route.ts call site
+provides:
+  - getBlastRadius(input, options?) with optional per-tenant client injection and cache-scope namespacing
+  - Swallowed searchDeliveredMessages() error now surfaces as unavailable/lookup_failed instead of a false-clean ok
+  - Campaign detail route clamps dateWindow.end to Date.now() and resolves reports.company_id -> mimecast_tenants -> tenant-scoped client
+affects: [phishing-campaign-review, mimecast-integration]
+
+tech-stack:
+  added: []
+  patterns:
+    - "Optional client-injection param on a service function so a caller can override a module's default singleton client while preserving the no-injection code path unchanged"
+    - "Cache-key namespacing by an opaque cacheScope string to prevent cross-tenant collisions on shared cache infra"
+
+key-files:
+  created: []
+  modified:
+    - lib/services/mimecast-blast-radius.ts
+    - lib/services/mimecast-blast-radius.test.ts
+    - lib/services/mimecast-client.test.ts
+    - app/api/phishing/campaigns/[id]/route.ts
+
+key-decisions:
+  - "An injected tenant client bypasses the global isMimecastConfigured() gate entirely -- it carries its own credentials and is self-sufficient, matching the plan's explicit behavior spec"
+  - "Swallowed deliveredResult.error is converted to a thrown Error so the existing outer try/catch (which already correctly logs err.message only, never response bodies) handles it -- no new logging path introduced"
+  - "Tenant resolution query is scoped to enabled = true and ORDER BY id LIMIT 1, so a disabled or duplicate mimecast_tenants row for a company falls back to the global client rather than picking an unintended row"
+
+patterns-established:
+  - "Pattern: optional second options arg ({ client?, cacheScope? }) added additively to an existing exported function signature to avoid breaking BlastRadiusInput/BlastRadiusResult contract"
+
+requirements-completed: [BUG-1-future-end-date, BUG-2-multi-tenant-gap]
+
+duration: 5min
+completed: 2026-07-16
+---
+
+# Quick Task 260716-n46: Fix Mimecast Blast-Radius Date-Window + Multi-Tenant Bugs Summary
+
+**getBlastRadius now accepts an injected per-tenant MimecastClient and surfaces previously-swallowed search errors instead of a false "clean" zero-count; the campaign detail route clamps its date window to now and resolves each report's own company-specific Mimecast tenant before calling it.**
+
+## Performance
+
+- **Duration:** 5 min
+- **Started:** 2026-07-16T16:42:23-04:00
+- **Completed:** 2026-07-16T16:47:19-04:00
+- **Tasks:** 3
+- **Files modified:** 4
+
+## Accomplishments
+- Bug 1 (false clean on fresh campaigns): `getBlastRadius` now rethrows a swallowed `searchDeliveredMessages()` error (e.g. Mimecast's `err_track_and_trace_invalid_end_date`) as `{ status: 'unavailable', reason: 'lookup_failed', error }` instead of a confident zero-count `ok` — and the route-level fix (`Math.min(createdAt + 24h, Date.now())`) prevents the future end-date from ever being sent in the first place.
+- Bug 2 (multi-tenant gap, D-05): the campaign detail route now looks up `reports.company_id -> mimecast_tenants` (enabled = true) and, when a row exists, builds a tenant-scoped client via the already-implemented `getMimecastClientForTenant()` and threads it into `getBlastRadius` via a new optional `options` param — falling back to the global env-configured client when no company-specific tenant is registered.
+- Cache keys are now namespaced by `cacheScope` (company_id, or `'global'`) so two tenants querying the same synthetic message-id/composite key can never read each other's cached result (T-N46-02 mitigation).
+
+## Task Commits
+
+Each task was committed atomically (TDD tasks 1-2 follow red/green discipline):
+
+1. **Task 1: Add optional per-tenant client injection + swallowed-error surfacing to getBlastRadius**
+   - `4d54aba` test(260716-n46): add failing tests for tenant client injection + swallowed-error surfacing (RED)
+   - `7c724cc` feat(260716-n46): support per-tenant client injection + surface swallowed delivered-search errors (GREEN)
+2. **Task 2: Add getMimecastClientForTenant coverage**
+   - `12250c1` test(260716-n46): add coverage for getMimecastClientForTenant (coverage-only — function was already implemented per the plan's interfaces block, so tests passed immediately; no GREEN implementation step needed)
+3. **Task 3: Clamp date window (Bug 1) and resolve per-company tenant (Bug 2) in the campaign detail route**
+   - `9951e53` fix(260716-n46): clamp blast-radius date window and resolve per-company Mimecast tenant
+
+**Plan metadata:** (this SUMMARY.md commit, made by the executor per worktree-mode protocol)
+
+## Files Created/Modified
+- `lib/services/mimecast-blast-radius.ts` — `getBlastRadius(input, options?: { client?; cacheScope? })`; resolves client as `options?.client ?? (isMimecastConfigured() ? getMimecastClient() : null)`; cache key prefixed by scope; throws on `deliveredResult.error` so the outer catch converts it to `unavailable/lookup_failed`; module doc-comment (c) updated to describe per-tenant resolution as supported (no longer "known limitation").
+- `lib/services/mimecast-blast-radius.test.ts` — 3 new tests: injected tenant client bypasses `getMimecastClient()`; injected client runs fan-out even when `isMimecastConfigured()` is false; swallowed `error` field degrades to `unavailable/lookup_failed` with `setCachedData` not called. Also added `getMimecastClientMock.mockClear()` to `beforeEach` (pre-existing gap in mock hygiene that the new "not.toHaveBeenCalled()" assertions exposed).
+- `lib/services/mimecast-client.test.ts` — new `describe('getMimecastClientForTenant', ...)` block: returns a client exposing the fan-out methods, builds a new instance per call, doesn't affect the cached global singleton, defaults `base_url` when omitted. Fake credentials only (`tid`/`tsecret`).
+- `app/api/phishing/campaigns/[id]/route.ts` — added `company_id` to the reports SELECT/`ReportRow`/mapped-`reports` shape; added `MimecastTenantRow` interface; inside the `primaryReport` branch, queries `mimecast_tenants` by `company_id` (`enabled = true`, `ORDER BY id LIMIT 1`) and builds a tenant client via `getMimecastClientForTenant()` when a row is found, passed to `getBlastRadius` as `{ client, cacheScope: companyId }`; `dateWindow.end` now `new Date(Math.min(createdAt.getTime() + 24h, Date.now()))`.
+
+## Decisions Made
+- Followed the plan's explicit instruction that an injected tenant client is self-sufficient and bypasses the global `isMimecastConfigured()` gate (tenant carries its own credentials).
+- Did not add a redundant reports query for company_id — reused the existing `reportsRes` query per the plan's scope guard.
+- Did not touch other Mimecast call sites (e.g. `mimecast-sync-service.ts`) — scope guard honored.
+
+## Deviations from Plan
+
+**1. [Test hygiene, in-scope] Added `getMimecastClientMock.mockClear()` to the existing `beforeEach` in `mimecast-blast-radius.test.ts`**
+- **Found during:** Task 1 (writing the "injected client bypasses getMimecastClient" test)
+- **Issue:** The pre-existing `beforeEach` never reset `getMimecastClientMock`'s call count, so a `not.toHaveBeenCalled()` assertion on it failed due to accumulated calls from earlier tests in the same file, not from the code under test.
+- **Fix:** Added `getMimecastClientMock.mockClear()` alongside the other five mock resets already in that `beforeEach`.
+- **Files modified:** `lib/services/mimecast-blast-radius.test.ts`
+- **Verification:** All 9 tests in the file pass; existing tests' behavior/assertions unchanged.
+- **Committed in:** `4d54aba` (Task 1 RED commit)
+
+---
+
+**Total deviations:** 1 (test-infrastructure hygiene fix required to make the plan's own specified assertion pass correctly; no production-code scope creep).
+**Impact on plan:** None on shipped behavior — test-only change needed for the plan's own required test assertions to be meaningful.
+
+## Issues Encountered
+- An intermediate `git stash -u` was run in error while diagnosing an unrelated pre-existing test failure (see below) — this is a prohibited operation in worktree mode. It was caught immediately via the system's post-command file-state reminder. Recovery used only read-only `git show stash@{0}:<path>` (never `git stash pop/apply/drop`) to retrieve the stashed file content, which was then written back with the Write tool and verified byte-identical via `diff` before proceeding. The stash entry (`stash@{0}`) was left untouched/undropped in the stash list to avoid any further stash-subcommand risk; it is a stale duplicate of already-committed work and can be safely ignored or cleaned up by the repository owner outside this workflow.
+- `npx vitest run` (full suite) shows 2 pre-existing failures in `lib/services/analyzer/itglue-search.test.ts` unrelated to this plan's files — confirmed present both before and after this plan's edits, and unrelated to any file this plan touches. Logged to `deferred-items.md` in this directory per the executor's scope-boundary rule; left unfixed.
+
+## User Setup Required
+
+None — no external service configuration required.
+
+## Next Phase Readiness
+
+- Both bugs are fixed within the existing `BlastRadiusInput`/`BlastRadiusResult` contract; no consumers of `getBlastRadius` besides the campaign detail route need updates.
+- The stray `stash@{0}` entry left in this worktree's shared stash list (see Issues Encountered) is a duplicate of already-committed work in this branch and is safe to drop later; not cleaned up here to avoid any `git stash` subcommand risk during this session.
+- `deferred-items.md` in this directory documents the pre-existing, out-of-scope `itglue-search.test.ts` failure for future triage.
--- a/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md
+++ b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md
@ -0,0 +1,5 @@
+# Deferred Items — 260716-n46
+
+## Pre-existing, out-of-scope test failure
+
+`lib/services/analyzer/itglue-search.test.ts > itglueSearch > tolerates per-call failures (configurations errors, flex still returns)` fails on a clean checkout of this branch, before any of this plan's changes. Confirmed present in the full `npx vitest run` both before and after Task 1-3 edits; none of this plan's files (`lib/services/mimecast-blast-radius.ts`, `lib/services/mimecast-blast-radius.test.ts`, `lib/services/mimecast-client.test.ts`, `app/api/phishing/campaigns/[id]/route.ts`) touch `lib/services/analyzer/itglue-search.ts` or its test. Left unfixed per the executor scope boundary (only auto-fix issues directly caused by the current task's changes).