feat(mail,store,api): open tracking — customerViewedAt, config-gated, default OFF (HT-32) - #30

src/db/migrate.ts (1)

301-320: 🚀 Performance & Scalability | 🔵 Trivial

Consider NOT VALID + VALIDATE CONSTRAINT for the new CHECK on a live table.

The constraint logic itself is correct — (direction = 'outbound') OR (customer_viewed_at IS NULL) properly restricts the new column to outbound threads. However, ALTER TABLE ... ADD CONSTRAINT ... CHECK (...) without NOT VALID scans and validates every existing row while holding an ACCESS EXCLUSIVE lock on threads, blocking all reads/writes for the duration. Since threads is an actively written table (delivery leases, replies), this could cause a noticeable stall in production during deploy.

Splitting into ADD CONSTRAINT ... CHECK (...) NOT VALID; followed by a separate VALIDATE CONSTRAINT ...; (which only takes SHARE UPDATE EXCLUSIVE) avoids blocking concurrent reads/writes. Since every existing row is NULL (the column is brand new), validation is cheap either way — the main win is the lock mode.

🗄️ Suggested migration change

 const MIGRATION_008_CUSTOMER_VIEWED_AT = `
 ALTER TABLE threads ADD COLUMN customer_viewed_at timestamptz;
 ALTER TABLE threads ADD CONSTRAINT threads_customer_viewed_at_outbound_only CHECK (
   (direction = 'outbound') OR (customer_viewed_at IS NULL)
-);
+) NOT VALID;
+ALTER TABLE threads VALIDATE CONSTRAINT threads_customer_viewed_at_outbound_only;
 `

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db/migrate.ts` around lines 301 - 320, Update
MIGRATION_008_CUSTOMER_VIEWED_AT to add threads_customer_viewed_at_outbound_only
with NOT VALID, then issue a separate ALTER TABLE VALIDATE CONSTRAINT statement.
Preserve the existing CHECK expression and constraint name while using the
lower-lock validation flow for the live threads table.

src/api/index.ts (1)

109-141: 🩺 Stability & Availability | 🔵 Trivial

Correct uniform-response contract; consider rate limiting the unauthenticated pixel route.

The gif-always-200/no-store/try-catch behavior matches spec §4g exactly (no validity/existence leak, store failure never surfaces past the gif). Since this is deliberately the API's one unauthenticated surface, consider adding rate limiting/abuse monitoring at the edge (e.g., reverse proxy or gateway) so it can't be used to flood recordThreadView calls or probe for load-based signals, especially since it's matched before any auth check.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/index.ts` around lines 109 - 141, Preserve the uniform
unauthenticated response behavior in the open-tracking pixel branch, and add
rate limiting or abuse monitoring for this route at the edge rather than
changing its response or authentication flow. Target requests identified by
matchOpenTrackingPixel, ensuring controls limit potential recordThreadView
flooding without exposing token validity or store errors.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes