docs(260718-7v8): pre-dispatch plan for Mimecast blast-radius held-message fix
This commit is contained in:
parent
4a77b0544c
commit
36f4d418d0
1 changed files with 185 additions and 0 deletions
|
|
@ -0,0 +1,185 @@
|
|||
---
|
||||
phase: quick-260718-7v8
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- lib/services/mimecast-client.ts
|
||||
- lib/services/mimecast-blast-radius.ts
|
||||
- lib/services/mimecast-blast-radius.test.ts
|
||||
- lib/services/mimecast-client.test.ts
|
||||
autonomous: true
|
||||
requirements: [BUG-BLAST-RADIUS-HELD]
|
||||
must_haves:
|
||||
truths:
|
||||
- "getHeldMessages() accepts optional start/end and threads them into data[0] as siblings of admin/searchBy"
|
||||
- "getBlastRadius() passes the SAME start/end strings used for searchDeliveredMessages into getHeldMessages()"
|
||||
- "Held rows whose sender domain does not match input.sender's domain are excluded from held/matched counts"
|
||||
- "A sender-domain-mismatched held row NEVER appears in perRecipient and never overrides a delivered status"
|
||||
- "A held row whose sender domain matches input.sender still counts and still overrides a same-recipient delivered entry"
|
||||
artifacts:
|
||||
- path: "lib/services/mimecast-client.ts"
|
||||
provides: "getHeldMessages with optional start/end date-window params"
|
||||
contains: "start"
|
||||
- path: "lib/services/mimecast-blast-radius.ts"
|
||||
provides: "date-scoped held lookup + sender-domain relevance guard"
|
||||
contains: "getHeldMessages"
|
||||
- path: "lib/services/mimecast-blast-radius.test.ts"
|
||||
provides: "coverage for date-scoping and sender-relevance filtering"
|
||||
- path: "lib/services/mimecast-client.test.ts"
|
||||
provides: "coverage that start/end thread into the request body"
|
||||
key_links:
|
||||
- from: "lib/services/mimecast-blast-radius.ts"
|
||||
to: "client.getHeldMessages"
|
||||
via: "start/end args derived from input.dateWindow"
|
||||
pattern: "getHeldMessages\\("
|
||||
---
|
||||
|
||||
<objective>
|
||||
Fix Mimecast blast-radius held-message false positives. Two locked fixes:
|
||||
|
||||
1. **Date-scope the held-message lookup** — `MimecastClient.getHeldMessages()` currently queries a recipient's ENTIRE hold queue regardless of date. Add optional `start`/`end` params threaded into the request body, and have `getBlastRadius()` pass the same window it already computes for `searchDeliveredMessages`.
|
||||
|
||||
2. **Sender-relevance guard** — even within the date window, unrelated held messages (different senders that happen to land in the same 24h window) must not count toward `held`/`matched` or override a recipient's `delivered` status. Filter held rows to those whose sender domain plausibly matches the campaign's `input.sender` domain (exact-or-proper-subdomain, case-insensitive) before they contribute or override.
|
||||
|
||||
Root cause was confirmed via two live verification scripts against the real Mimecast tenant: date-scoping alone dropped the false held count from 15 → 1, but that 1 remaining row was still an unrelated sender — so the sender-domain guard is required in addition.
|
||||
|
||||
Purpose: The Blast Radius panel shows the correct per-recipient status (delivered, not a misleading held) for reported campaigns.
|
||||
Output: Modified client method + blast-radius wiring, extended test coverage.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@./CLAUDE.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Extracted from codebase — executor should use these directly, no exploration needed. -->
|
||||
|
||||
From lib/services/mimecast-client.ts (current getHeldMessages signature to extend):
|
||||
```typescript
|
||||
async getHeldMessages(options: {
|
||||
recipient?: string;
|
||||
maxMessages?: number;
|
||||
} = {}): Promise<{ messages: MimecastHeldMessage[]; totalCount: number }>
|
||||
```
|
||||
Inside the do-loop the request body is built as:
|
||||
```
|
||||
const reqBody: any = { admin: true };
|
||||
if (options.recipient) {
|
||||
reqBody.searchBy = { fieldName: 'recipient', value: options.recipient };
|
||||
}
|
||||
```
|
||||
The 403 fallback path (`fallbackBody`) spreads `{ ...reqBody }`, so any new fields added to reqBody are carried into the fallback automatically.
|
||||
|
||||
From lib/services/mimecast-client.ts — searchDeliveredMessages, the date-format precedent to mirror. The CALLER pre-formats start/end; the method receives them as strings:
|
||||
```typescript
|
||||
async searchDeliveredMessages(options: {
|
||||
to?: string; from?: string; subject?: string;
|
||||
startHours?: number; start?: string; end?: string; route?: string;
|
||||
}): Promise<{ messages: MimecastDeliveredMessage[]; error?: string }>
|
||||
```
|
||||
|
||||
MimecastHeldMessage shape (sender is already normalized into `from`):
|
||||
```typescript
|
||||
export interface MimecastHeldMessage {
|
||||
id: string; subject: string; from: string; fromDisplay: string;
|
||||
to: string; toDisplay: string; dateReceived: string; reason: string;
|
||||
reasonCode: string; policyInfo: string; route: string;
|
||||
hasAttachments: boolean; size: number;
|
||||
}
|
||||
```
|
||||
|
||||
From lib/services/mimecast-blast-radius.ts — the caller already computes the window strings (lines 118-119) and calls getHeldMessages WITHOUT them (line 131):
|
||||
```typescript
|
||||
const startStr = input.dateWindow.start.toISOString().replace(/\.\d{3}Z$/, '+0000');
|
||||
const endStr = input.dateWindow.end.toISOString().replace(/\.\d{3}Z$/, '+0000');
|
||||
// ...
|
||||
client.getHeldMessages({ recipient: input.recipient }),
|
||||
```
|
||||
The per-recipient merge loop (lines 172-180) applies delivered → rejected → held in order; held currently overwrites unconditionally.
|
||||
|
||||
From lib/services/campaign-classifier.ts — comparison LOGIC to mirror (do NOT reuse this function directly; it is allowlist-specific):
|
||||
```typescript
|
||||
// Exact-domain-or-proper-subdomain match ONLY — never .includes() substring.
|
||||
lower === allowed || lower.endsWith(`.${allowed}`)
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Date-scope getHeldMessages and add sender-relevance guard in getBlastRadius</name>
|
||||
<files>lib/services/mimecast-client.ts, lib/services/mimecast-blast-radius.ts</files>
|
||||
<behavior>
|
||||
- getHeldMessages({ recipient, start, end }) sends start/end inside data[0] alongside admin/searchBy when provided; omits them when not provided (backward compatible).
|
||||
- The 403 fallback body still includes start/end (it spreads reqBody).
|
||||
- getBlastRadius passes startStr/endStr (the same window already computed for searchDeliveredMessages) into getHeldMessages.
|
||||
- Held rows whose sender domain does NOT match input.sender's domain (exact-or-proper-subdomain, case-insensitive) are dropped BEFORE computing held/matched counts and BEFORE the perRecipient merge.
|
||||
- A dropped (mismatched) held row must never appear in perRecipient and must never override a delivered entry.
|
||||
- A held row whose sender domain matches still counts toward held/matched and still overrides a same-recipient delivered entry (unchanged behavior for relevant rows).
|
||||
</behavior>
|
||||
<action>
|
||||
In lib/services/mimecast-client.ts, extend the `getHeldMessages` options object to add optional `start?: string` and `end?: string`. Inside the do-loop, after building `reqBody` with `admin`/`searchBy`, set `reqBody.start = options.start` and `reqBody.end = options.end` only when each is present (siblings of `admin` and `searchBy`, exactly the shape confirmed against Mimecast's docs: `{ admin: true, start, end, searchBy: {...} }`). Do NOT format dates inside the method — the caller passes pre-formatted strings, mirroring how `searchDeliveredMessages` receives `start`/`end` today. The existing 403 `fallbackBody` spreads `{ ...reqBody }`, so start/end propagate automatically — leave that path structurally unchanged.
|
||||
|
||||
In lib/services/mimecast-blast-radius.ts:
|
||||
(a) Change the `client.getHeldMessages({ recipient: input.recipient })` call to also pass `start: startStr, end: endStr` (the same strings already computed on lines 118-119 for the delivered search — do not compute a second window).
|
||||
(b) Add a small local helper `domainsMatch(a: string, b: string): boolean` near the other module-local helpers (isRejectedStatus/isClickEvent). It extracts the domain after the last `@` from each address, lowercases both, and returns true when the two domains are equal OR one is a proper subdomain of the other (`x === y || x.endsWith('.'+y) || y.endsWith('.'+x)`). Mirror the comparison logic from campaign-classifier's `domainMatchesAllowlist` (case-insensitive, exact-or-proper-subdomain, never bare substring). Guard against missing `@` / empty domain by returning false. Do NOT import or reuse `domainMatchesAllowlist` itself.
|
||||
(c) After `const heldRows = heldResult.messages ?? []`, derive `const relevantHeldRows = heldRows.filter((h) => domainsMatch(h.from, input.sender))`. Use `relevantHeldRows` everywhere `heldRows` currently feeds counts and the perRecipient merge: `const held = relevantHeldRows.length;` and the `for (const row of heldRows)` perRecipient loop iterates `relevantHeldRows`. `matched` stays `delivered + held` (now the relevance-filtered held count).
|
||||
(d) Optionally (Claude's discretion) add a `console.debug('[MIMECAST-BLAST-RADIUS] held rows filtered as unrelated sender:', heldRows.length - relevantHeldRows.length)` when the counts differ, matching the file's existing debug-log precedent.
|
||||
Do not touch caching, the delivered/rejected split, threat-event logic, or the injected-client path.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /opt/stacks/pulse && npx tsc --noEmit --pretty 2>&1 | grep -E "mimecast-(client|blast-radius)\.ts" | grep -v '^#' | grep -c error | grep -qx 0 && echo TYPECHECK_OK</automated>
|
||||
</verify>
|
||||
<done>getHeldMessages accepts start/end and threads them into data[0]; getBlastRadius passes the shared window and filters held rows by sender-domain relevance before counting/merging; tsc clean for both files.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Extend test coverage for date-scoping and sender-relevance filtering</name>
|
||||
<files>lib/services/mimecast-blast-radius.test.ts, lib/services/mimecast-client.test.ts</files>
|
||||
<behavior>
|
||||
- blast-radius test: an in-window held row from an UNRELATED sender domain is excluded from held/matched and never appears in perRecipient; a delivered row for that recipient keeps status 'delivered'.
|
||||
- blast-radius test: a held row whose sender domain MATCHES input.sender still counts and still overrides a same-recipient delivered entry to 'held'.
|
||||
- blast-radius test: getHeldMessages is called with start/end equal to the same formatted window passed to searchDeliveredMessages.
|
||||
- client test: getHeldMessages includes start/end in the POST body data[0] when provided, and omits them when not provided.
|
||||
</behavior>
|
||||
<action>
|
||||
In lib/services/mimecast-blast-radius.test.ts (mocks already stub the client — follow the existing `getHeldMessagesMock` pattern):
|
||||
- Add a test proving sender-relevance filtering: mock a delivered row for `BASE_INPUT.recipient` (from `BASE_INPUT.sender`, status 'Delivered') AND a held row for the SAME recipient but `from: 'promo@paulfredrick.test'` (unrelated domain). Assert the result has `held: 0`, `delivered: 1`, `matched: 1`, and `perRecipient` contains `{ recipient: BASE_INPUT.recipient, status: 'delivered' }` and NO 'held' entry.
|
||||
- Add a test proving a matching-domain held row still overrides: delivered row for the recipient from `BASE_INPUT.sender`, plus a held row for the same recipient with `from` sharing `BASE_INPUT.sender`'s domain (e.g. `queue@evil.test`). Assert that recipient's status is 'held' and `held: 1`.
|
||||
- Add an assertion (can extend an existing merge test) that `getHeldMessagesMock` was called with an object whose `start`/`end` equal the strings passed to `searchDeliveredMessagesMock` (capture via `expect.objectContaining({ recipient, start: <same>, end: <same> })`, or read `getHeldMessagesMock.mock.calls[0][0]`). The expected format is `input.dateWindow.start.toISOString().replace(/\.\d{3}Z$/, '+0000')`.
|
||||
- The existing "merges delivered/held/threat-event fixtures" test uses a held row whose `from` is `BASE_INPUT.sender` (domain matches) — confirm it still passes unchanged (held: 1, matched: 2).
|
||||
|
||||
In lib/services/mimecast-client.test.ts:
|
||||
- getHeldMessages is a network method, so add a focused unit test that stubs the private request path. Follow the existing factory-test discipline in this file: build a client via `getMimecastClientForTenant(FAKE_TENANT)` and spy on its `request` method (cast to any) with `vi.spyOn(client as any, 'request').mockResolvedValue({ data: [], meta: { pagination: {} } })`. Call `getHeldMessages({ recipient: 'r@x.test', start: '2026-07-14T00:00:00+0000', end: '2026-07-15T00:00:00+0000' })` and assert the request was called with a body whose `data[0]` contains `start` and `end` matching those strings alongside `admin` and `searchBy`. Add a second call without start/end and assert `data[0]` has NO `start`/`end` keys. Import `vi` if not already imported.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /opt/stacks/pulse && npx vitest run lib/services/mimecast-blast-radius.test.ts lib/services/mimecast-client.test.ts 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<done>All new and existing tests in both files pass; date-scoping and sender-relevance behavior are covered.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `npx tsc --noEmit --pretty` clean for both modified source files.
|
||||
- `npx vitest run lib/services/mimecast-blast-radius.test.ts lib/services/mimecast-client.test.ts` all green.
|
||||
- Manual trace against the Seubert/Tomlinson case: with a ±24h window and the campaign sender, the 15 unrelated marketing holds (and the coincidental in-window Paul Fredrick hold) are excluded, leaving the recipient shown as 'delivered'.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- getHeldMessages() accepts optional start/end and threads them into data[0] (backward compatible when omitted).
|
||||
- getBlastRadius() date-scopes the held lookup with the same window as the delivered search.
|
||||
- Held rows are filtered by exact-or-proper-subdomain sender match against input.sender before counting/merging; mismatched rows never appear in perRecipient and never override delivered.
|
||||
- Test coverage added for both the date-threading and the sender-relevance guard.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/260718-7v8-fix-mimecast-blast-radius-held-message-f/260718-7v8-SUMMARY.md` when done
|
||||
</output>
|
||||
Loading…
Add table
Add a link
Reference in a new issue