feat(sending): deletion-resistant feedback provenance and subtype aggregation (B8)

[jiashuoz]

Slice B8 of the sending abuse prevention plan (ops PR #320, Task 8): deletion-resistant feedback provenance and subtype aggregation. Passive evidence capture only; the detector (B9) is not in this PR and no control is enabled.

What changes

Feedback counts after deletion. The SNS consumer runs a new delivery.FeedbackProcessor seam BEFORE it looks for a live message, in its own transaction; if accounting fails the notification is not acked and the provider retries. The seam (implemented by the sending-policy module) never reads messages, agent_identities, or users:

• correlates by the SES message id bound at settlement (normalized), then by the echoed X-E2A-Provider-Attempt marker (now parsed; shape-checked like the message-id marker);
• matches each recipient the event names against the keyed HMACs recorded at authorization — an address outside the authorized envelope is ignored (no accounting, no suppression);
• one row per provider event id, so a redelivered notification is a zero delta;
• derives the detector bucket from the full kind + retained subtypes: delivered, terminal_other (transient/undetermined bounce), hard_bounce (permanent, including global-list Suppressed), complaint. Account/tenant suppression-list subtypes (bounce or complaintSubType) are none but still repair the local list;
• moves evidence monotonically per recipient (none < delivered < terminal_other < hard_bounce < complaint): subtracts the prior bucket from the epoch/day it was counted in and adds the new one to the account's current outcome_epoch + ingestion UTC day on the correlation's immutable shared/dedicated path; equal rank is a zero delta with provider time then event id deciding stored provenance; lower evidence never regresses;
• repairs the account suppression from the signed event's plaintext recipient only while the account exists; after account deletion it updates retained provenance only.

Suppression upsert ownership → internal/suppressionsync. The upsert advances sync_generation and clears removal_pending; a removal that observed an earlier generation deletes nothing. identity.AddSuppressionTx routes through it (existing rows keep their first reason/source). The consumer uses the seam's Inserted verdict for the suppression_added event so a second upsert cannot hide a genuine insert.

Retention. Account deletion stamps the post-deletion horizon (policy sending_feedback_post_account_retention_days, default 30) on the account's correlations and events. An hourly sending_feedback_maintenance job removes expired provenance and daily outcome rows older than the detector window + 1 day.

Keyring coverage is a startup gate. The server refuses to start if any unexpired recipient row was signed under a key version the keyring does not hold (rotation is superset-first).

Tests

• sendingpolicy: bucket/rank table; purge message + agent then feedback by SES id and by attempt marker with HMAC match and a forged recipient rejected; monotonic evidence (delivered→complaint moves the unit, delayed delivery/suppression-subtype ignored, equal-rank zero delta, hard bounce kept across a suppression-list bounce, terminal_other denominator); shared vs dedicated aggregates incl. a HITL notification on the shared path; post-account-deletion provenance only; keyring rotation (superset matches, narrow keyring fails coverage and cannot match, expiry releases the version); GC.
• suppressionsync: upsert generation + stale-removal race.
• delivery: processor runs first and for uncorrelated events, processor error fails Process, Inserted verdict drives the event, parser retains subtypes and drops a malformed marker.
• identity: account deletion stamps retention on that account's rows only.
• outbound: the attempt header name is pinned equal across the adapter and the parser.

Plan step 5 (go test -p 1 -race over delivery/sendingpolicy/suppressionsync) green locally.

Review round 1 (correctness + adversarial, Opus) → second commit

Both reviewers found the same blocker: the seam wrote the suppression in its own transaction, so a live transaction that failed after that commit lost suppression_added and its lifecycle transition permanently on the SNS retry (the seam returned Duplicate with no suppressions, the store then reported added=false), and every feedback suppression lost source_message_id and changed reason on GET /v1/account/suppressions.

Fixed by giving ownership back to the live path. The seam does accounting only and reports RepairNeeded; when a message survives, the consumer writes the suppression in its own transaction exactly as before (same fields, same event semantics, insert atomic with the announcement). Only when no message can own the row does the consumer call RepairSuppressions, and repair is limited to customer messages so a bounced approval notice cannot suppress the account owner's own address. A regression test drives the failure-then-retry and asserts exactly one event.

Also applied: retention janitor reads the effective policy (a database-source deployment could otherwise delete evidence at the config window); the keyring gate skips a deployment with no keyring and asks a bounded question instead of scanning two unindexed tables at every boot; migration 121 indexes the three feedback access paths; feedback recipients are addr-spec extracted before the HMAC match (a DSN Final-Recipient with a display name previously failed silently) and unmatched recipients are counted and logged; suppression-list subtypes compare case-insensitively; the correlation lookup prefers an unexpired row deterministically; the account-deletion lock-order interaction is documented. New tests: retry, purged-message repair, equal-rank provenance tie-break (an earlier straggler must not overwrite), repair guard on a deleted account.

Not changed, called out instead: the suppressionsync generation guard has no production remover yet (RemoveSuppression is still an unconditional delete) — it is Task 11 groundwork, and the design addendum now says so.

Review round 2 (mutation-tested re-review, Opus) → third commit

Verdict: no blockers. Both round-1 blockers proved closed by mutation (breaking the fix fails a named test in each case), and the equal-rank tie-break gap from round 1 is closed too. Its should-fixes are applied:

• A repaired suppression is now announced. The purged-message path fires suppression_added with no message id (the schema documents that field as present only when known). Silently adding a row to the customer's suppression list was the one genuinely new customer-visible behavior in the slice.
• Addresses are normalized once, in the parser. The reviewer showed the seam and the live path could disagree on a decorated recipient — crediting the detector under the real address while suppressing a literal Bob <bob@x.test> the customer could never clear. The DSN Final-Recipient address-type prefix is stripped too.
• Two proven-uncovered properties now have tests: the stored suppression reason/source (mutating the diagnostic to a constant passed the whole suite before) and platform mail accounting-but-not-repairing (mutating the purpose guard passed before). Both fail under those mutations now.
• Smaller: RepairSuppressions takes FOR KEY SHARE on the account instead of a bare EXISTS (a concurrent deletion was a constraint violation, not a no-op); the unmatched-recipient log fires once per event rather than per redelivery; the config-source policy read no longer opens a transaction; dead helper removed.
• Migration 121 drops the recipients index. Its only reader is the keyring gate's "version not in this set" predicate, which no btree can serve — measured on 200k seeded rows, the planner picks a sequential scan even with enable_seqscan=off. It was write amplification on the hottest of these tables with no reader. The remaining three indexes are justified, and the comment now states the gate's real cost and that non-concurrent CREATE INDEX takes SHARE.

Coverage audit → fourth commit

A pass over the new code found two things with no coverage at all, both in the category where a failure is silent:

• The retention janitor was at 0% — the one component here that deletes evidence, and one a reviewer had already caught reading the wrong policy source. Now covered: a database-source deployment cuts at the policy row's window rather than the config file's (reverting that read fails the test), an expired correlation takes its recipients and events with it while a retained one survives, and the periodic reaches River's maintenance queue.
• Both production wiring lines were unasserted. Installing the accounting seam on the delivery consumer and registering the janitor were one line each in main.go. Dropping either leaves the whole suite green while the feature is inert in production, because the consumer still acks every notification and still runs the message lifecycle — the detector just never receives evidence. Both now compose through the outbound root and are pinned by a wiring test that fails when the root forgets them.

Function coverage of the new code after this: decision logic (bucket derivation, ranking, subtype exclusion) 100%; the stateful paths 77–85%, with the remainder being database-failure branches; the janitor 67–100%, up from 0%.

Gate

B8's gate is ingestion observation, and it has to run on prod, not staging: config.staging.yaml deliberately omits delivery_feedback:, so SES publishes no delivery/bounce/complaint events there and nothing would arrive to observe. The keyring is wired in both hosted environments (E2A_SENDING_FEEDBACK_HMAC_KEYS in the env assembler and both compose files), so recipient provenance is written on both today.

After the release that carries this slice, confirm on prod that SES feedback for real sends lands in sending_feedback_events / sending_feedback_recipients with buckets set, that account_sending_outcomes_daily accumulates, that the hourly maintenance job runs, and that the unmatched-recipient log line stays quiet (a systematic normalization or key mismatch is exactly what it exists to surface). No activation, no detector.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX