fix(email): support multi-recipient inbound delivery and eliminate ghost threads

[wg2038]

Summary

Fixes #198.

Inbound email delivery suffered from several interlocking issues when delivering to multiple recipients or across tenants:

1. Global index vs tenant isolation: uniq_email_messages_smtp_id was unique globally on LOWER(smtp_message_id). When an email was addressed to recipients across multiple companies, the second company's insert was rejected with duplicate key violation.
2. Same-tenant multiple recipients: recipients did not deduplicate addresses ([...to, ...cc]), and the fanout looped per recipient instead of per company. When multiple recipients belonged to the same company, iteration 1 inserted the message and iteration 2 attempted to insert the same smtp_message_id into the same company, causing a duplicate key error falsely logged as race_dedup.
3. Ghost threads: findOrCreateEmailConversation committed a conversation in its own transaction before persistEmailMessage. When persistence failed, an empty conversation with 0 messages was left behind indefinitely.
4. False 550 bounces: If all attempted inserts hit the constraint or were dropped, the handler returned 404, causing email-gate to issue a permanent 550 bounce ("No such recipient") for valid mailboxes.

Key Changes

• Migration 0005 (0005_email_messages_company_smtp_id):
  • Drops global index uniq_email_messages_smtp_id.
  • Creates company-scoped index uniq_email_messages_company_smtp_id on (company_id, LOWER(smtp_message_id)) WHERE smtp_message_id IS NOT NULL.
  • Bumps schema manifest and version bounds to 5.
• Inbound Handler Refactor (server/src/api/inbound-email.ts):
  • Deduplicates recipient addresses from To and Cc.
  • Resolves recipients upfront and groups by companyId.
  • Returns 404 immediately if no recipient resolves to any tenant before uploading attachments.
  • Pre-checks whether all target tenants have already received the message for fast idempotent responses.
  • Fans out per distinct company (single iteration per tenant), adding all recipients of that tenant to memberIds.
  • Automatically deletes empty conversations if conv.created === true and message persistence fails, eliminating ghost threads.
  • On concurrent race collisions, attaches to the winning message instead of failing.
  • Returns 500 on unexpected persist failure instead of 404 to trigger MTA tempfail retry instead of permanent bounce.
• Integration Tests (server/src/__integration__/inbound.test.ts):
  • Added tests for multi-recipient delivery in the same company, cross-tenant multi-company delivery with identical Message-ID, To/Cc deduplication, and ghost conversation rollback.

Verification

• npm run lint: 0 warnings, 0 errors.
• npm run typecheck & npm run server:typecheck: passed.
• npm run guard:big-brain, guard:llm-tracked, guard:engine-registry: passed.
• node --import tsx --test server/src/__integration__/inbound.test.ts: all 16 tests passed.
• node --import tsx --test server/src/__integration__/schema-migrations.test.ts: all 4 tests passed.
• npm run test: all 1181 unit tests passed.
• npm run test:integration: all 323 integration tests passed.

[yetone]

Reviewed — this is a good fix and I want it. The diagnosis is the part I'd highlight: uniq_email_messages_smtp_id being globally unique on LOWER(smtp_message_id) meant a single email addressed across two tenants had the second tenant's insert rejected as a duplicate, and the failure then surfaced as a 550 permanent bounce for a perfectly valid mailbox. A tenant-scoped index is the right correction, and returning 500 rather than 404 on an unexpected persist failure is what turns that into a retry instead of a lost mail.

It pairs well with #216, which I merged today — that one stops the worker turning upstream 5xx into a hard bounce. Together they close both halves of #198/#200's "valid mail permanently bounced" story.

I can't merge it right now: the branch is CONFLICTING / DIRTY. I merged six more PRs today (#224, #216, #215, #177, #187, #222), several into server/src/api/ and the migration manifest, so the conflict is mine. Please rebase onto current main.

Two things to check while rebasing:

• The manifest bump. You add 0005 and bump the version bounds to 5. feat(agent-routing): elect one agent for unaddressed human group messages #123 also adds a 0006. Whichever lands second has to renumber, so after the rebase please re-confirm your migration number is still free and schema-migrations.test.ts agrees.
• The index swap on a live table. Dropping uniq_email_messages_smtp_id and creating the company-scoped replacement is fine logically, but please confirm the create is CONCURRENTLY or that email_messages is small enough that a brief lock is acceptable — production takes real inbound mail on this table.

One heads-up on timing, not on this PR: production is currently blocked on migration 0002 (its precheck fails against live data, which is why nothing has deployed since 2026-09-03). So 0005 won't reach production the moment it merges — it'll go out in the same catch-up run once 0002 is cleared. Worth knowing so the rollout isn't a surprise.

Rebase and I'll take it.

[wg2038]

Rebased onto latest main (97b39bf) and addressed both points:

1. Manifest & Migration Renumbering: Renumbered the migration to 0006 (0006_email_messages_company_smtp_id), bumped MIN_SUPPORTED_SCHEMA_VERSION and MAX_SUPPORTED_SCHEMA_VERSION to 6, and updated schema-migrations.test.ts accordingly.
2. Zero-downtime Concurrent Index Swap:
   • Both CREATE UNIQUE INDEX CONCURRENTLY and DROP INDEX CONCURRENTLY run outside a transaction block (transactional: false).
   • Uses ensureConcurrentIndex to safely build the new tenant-scoped index uniq_email_messages_company_smtp_id before dropping the old global index, guaranteeing continuous index coverage and zero table write locks for incoming production email.
   • Added uniq_email_messages_company_smtp_id to REQUIRED_SCHEMA_INDEXES for promotion verification.

All 7 CI checks are green and the branch is clean and MERGEABLE!

[yetone]

Re-reviewed after the rebase — both points are addressed properly, and I verified the parts that carry risk rather than taking the description on trust:

• Migration order is safe on a fresh database too. The frozen baseline (DDL, migration 0001) still creates the global uniq_email_messages_smtp_id, and it is applied once by the migration owner — not re-run on every boot — so 0006's DROP INDEX CONCURRENTLY is not undone by a later ensureSchema(). Version order guarantees create-then-drop on new and existing databases alike.
• The index swap cannot fail on live data. (company_id, LOWER(smtp_message_id)) is strictly weaker than the global LOWER(smtp_message_id), so the CONCURRENTLY build has no existing rows that could violate it, and coverage is continuous because the new index is built before the old one is dropped.
• Nothing infers the dropped index. No ON CONFLICT targets it; the only reference was the duplicate-key message match in inbound-email.ts, and you kept the legacy name in that regex, which is the right call for the window where a replica is still running the old build.
• The ghost-conversation cleanup won't trip an FK. conversation_members_conversation_fk is ON DELETE CASCADE, and findOrCreateEmailConversation returns created: true only on the insert path, so the NOT EXISTS (SELECT 1 FROM messages …) guard is the correct narrow condition.
• The dedup pre-check is now per-tenant (allAlreadyDelivered), and a partially-delivered message correctly proceeds for the tenants still missing it while reusing the existing delivery for the rest. The race-loser path resolving the winner's row and pushing it into inserts is what keeps a concurrent double-delivery from falling through to the 500.

Merging. One rollout note, repeating what I said above so it isn't a surprise: this bumps both schema bounds to 6, so it won't reach production until migration 0002's precheck is cleared — nothing has deployed since 2026-09-03. It'll go out in the same catch-up run.

Thanks for the clean rebase.

https://claude.ai/code/session_0126NkM9crkemuV6Ho4LWv59