feat(email): add org_email_domains claim/verify/remove

[cwaits6]

Summary

Phase 5 · PR 6 — adds org_email_domains, the table and admin routes that let an org claim and verify a custom sending domain through Resend. No change to how mail is actually sent (lib/email/identity.ts, resend.ts, serving.ts are untouched) — that's PR 7.

Changes

• Migration (supabase/migrations/20260819000000_org_email_domains.sql): org_email_domains table with org_id default app_current_org_id(), restrictive isolation policy (org_id = (select app_request_org_id())), a permissive admin-only policy, a domain_shape CHECK, a unique-per-org index, and column-level GRANTs so admins can write the domain but not status / verified_at — only the service role can.
• POST /api/admin/email-domain: service-role claim route. Calls Resend domains.create, inserts the row, rolls back the Resend domain if the insert fails. Org anchored on the caller's RLS-scoped profile.
• DELETE /api/admin/email-domain: remove route, request-client only (no service role, no Resend domains.remove — per decision D5).
• POST /api/admin/email-domain/verify: service-role verify route. Calls Resend domains.verify + domains.get, persists status, records, verified_at, last_checked_at.
• Admin UI (app/admin/settings/email/page.tsx): claim form, status card, DNS record list, Verify / Remove actions. Linked from app/admin/settings/page.tsx.
• docs/security/service-role-inventory.md: two new rows under "App routes and pages" for the claim/verify routes.
• pgTAP (supabase/tests/org_email_domains_suite.sql, 39 assertions): cross-org isolation, non-admin write rejection, own-org lifecycle, the column-level grant matrix, and constraint checks. tenancy_leak_suite.sql gets a fixture row so the completeness gate covers the new table.
• lib/supabase/database.types.ts: regenerated, trimmed to only the org_email_domains hunk (unrelated local-stack strays from a parallel branch were dropped).

Deviations from plan (see docs/plans/phase-5-domains-email.md §10.1/§10.2/§12 step 6, decision D5)

1. status CHECK vocabulary widened to the union of the plan's spec (not_started, pending, verified, failure, temporary_failure) and the installed resend@6.20.0 SDK's DomainStatus type (pending, verified, failed, not_started, partially_verified, partially_failed). A real Resend status can never trip the CHECK and strand the verify route with a 500; anything outside both sets still fails loudly. pgTAP asserts a made-up status is rejected. Nothing gates on any value but verified.
2. Migration applied locally via raw psql, not supabase migration up — the shared local stack already carries 20260818000000_reserved_org_slugs from the parallel Phase 5 · PR 1 — reserved slug labels denylist in provision_organization() + lib/org.ts #358 branch, which this branch doesn't have, and migration up refused with LegacyMigrationMissingLocalError. Applied the file directly with psql -v ON_ERROR_STOP=1 and recorded the version row in supabase_migrations.schema_migrations, same bookkeeping migration up would do. CI applies the file normally. If Phase 5 · PR 1 — reserved slug labels denylist in provision_organization() + lib/org.ts #358 merges first, 20260819000000 should be re-timestamped to sort after it.
3. npm run db:types output hand-trimmed — the generated diff also carried unrelated local-stack strays (a payment_handles table, custom_handle nullability); only the org_email_domains block was kept.
4. Inventory row format — moved "POST handler:" into the description column instead of inline after the path, to satisfy the guard's row regex.

Validation

Check	Result
npx tsc --noEmit	✅
npm run build	✅ (new routes present: /admin/settings/email, /api/admin/email-domain, /api/admin/email-domain/verify)
npm run lint (--max-warnings=0)	✅
npm run guard:tenancy	✅ inventory in sync (22 call sites)
npx vitest run	✅ 228 passed, 0 failed (16 files)
pgTAP org_email_domains_suite.sql (local, docker exec)	✅ 39/39
pgTAP tenancy_leak_suite.sql	✅ 96/96, incl. fixture completeness + cross-org read denial for org_email_domains
pgTAP schema_tenancy_lint.sql	✅ 35/35
pgTAP idor_suite.sql, branding_rls_suite.sql	✅ no failures

Unauthenticated smoke: all three routes → 401, /admin/settings/email → 307 to login. Authenticated smoke of the claim route was deliberately not run — it would create a real domain in the Resend account tied to local .env.local. Local pgTAP ran against the shared stack (which also carries #358's migration); CI's ephemeral pgtap job is the authoritative gate.

Out of scope / next steps

• No Resend domains.remove call on remove, and no scheduled re-verification (per decision D5).
• No change to actual mail sending — that's PR 7.

Fixes #363

[linear-code]

CWA-70

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
two42	Ready	Preview	Aug 19, 2026 4:42am

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 70b3369.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e6594ac6-645e-4d6b-a8cf-807d47d72735

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[cwaits6]

🔍 Comprehensive PR Review

PR: #367
Reviewed by: 5 specialized agents (code-review, error-handling, test-coverage, comment-quality, docs-impact)
Date: 2026-08-19

---

Summary

The tenancy/RLS layer on org_email_domains is correct and well-tested — the restrictive isolation policy, the column-level GRANT matrix that blocks admin self-verification, and service-role org anchoring were all independently verified by multiple agents. The gaps are in operational robustness and coverage: the claim route can leave a Resend-side domain orphaned and unlogged if the post-create DB write fails, both new routes lack the try/catch every other Resend-calling route in this codebase uses, none of the four new application-layer files have any test coverage, and one plan-doc section — the literal spec for the next PR in this feature — now states a factually wrong status count.

Verdict: REQUEST_CHANGES

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	5
🟡 MEDIUM	4
🟢 LOW	5

---

🟠 High Issues

1. Resend domain orphaned, unlogged, and unrecoverable if the post-create DB update fails

📍 app/api/admin/email-domain/route.ts:142-165

The claim route's insert-failure branch rolls back correctly (deletes the row, scoped to id+org_id), but the update-failure branch three lines below it (persisting resend_domain_id/status/dns_records) doesn't — rd.id is never logged and the Resend-side domain is never removed. Retries 409 forever with no trail to find the orphaned resource.

View fix

if (updateError || !saved) {
  console.error(
    "email-domain create: scoped update failed (org=%s, id=%s, resend_domain_id=%s):",
    orgId, inserted.id, rd.id, updateError,
  );
  const { error: resendCleanupError } = await getResend().domains.remove(rd.id);
  if (resendCleanupError) {
    console.error("email-domain create: Resend cleanup after failed update also failed (org=%s, resend_domain_id=%s):", orgId, rd.id, resendCleanupError);
  }
  const { error: rollbackError } = await service
    .from("org_email_domains").delete().eq("id", inserted.id).eq("org_id", orgId);
  if (rollbackError) {
    console.error("email-domain create: rollback delete after failed update also failed (org=%s, id=%s):", orgId, inserted.id, rollbackError);
  }
  return NextResponse.json({ error: "Failed to save domain. Please try again." }, { status: 500 });
}

---

2. No top-level try/catch around Resend SDK calls

📍 app/api/admin/email-domain/route.ts:65-168, app/api/admin/email-domain/verify/route.ts:596-697

Every existing Resend-calling route (approve/route.ts, family-invites/route.ts) wraps the SDK call in try/catch, since network-level failures throw rather than returning { error }. Neither new route does — a thrown exception bypasses this file's own logging, and in the claim route, bypasses the insert-rollback entirely.

Fix: wrap each handler body in a top-level try/catch matching approve/route.ts's shape; the claim route's catch should attempt the same insert-rollback used on the known-error paths.

---

3. Claim route's rollback-on-Resend-failure path has zero test coverage

📍 app/api/admin/email-domain/route.ts:478-507

The PR's own review scope calls this out as the thing to verify. No unit or pgTAP test exercises it — a future refactor that drops the org_id filter or inverts the condition would ship with every suite green.

Fix: add app/api/admin/email-domain/route.test.ts mocking createServiceClient() and resend, asserting the rollback delete().eq() chain fires on domains.create failure and doesn't on success.

---

4. New pure validation/formatting functions ship with zero unit tests

📍 app/api/admin/email-domain/route.ts:398-399 (DOMAIN_SHAPE), app/admin/settings/email/page.tsx:38-61

This codebase has an established pattern for testing exactly this class of code (lib/email/identity.test.ts). DOMAIN_SHAPE is the only gate between raw admin input and the Resend API call and has no boundary/adversarial-case tests.

Fix: export DOMAIN_SHAPE, add a table-driven test with boundary lengths, hyphens, empty labels, uppercase.

---

5. Plan doc's From:-address gate spec cites a stale status count

📍 docs/plans/phase-5-domains-email.md:921 (§10.3)

This PR widens the status CHECK from 5 values to 8 (Deviation 1). §10.3 — the literal spec for PR 7, the next step in this feature — still says "any of the four non-verified statuses" falls back to PLATFORM_ADDRESS. It's now seven of eight. A PR 7 implementer following this spec could hardcode an enumerated four-value allowlist instead of gating on equality to 'verified'.

Fix: reword §10.3 to gate on equality-to-'verified' rather than an enumerated count; update §10.1/§10.2's stale 5-value listings in the same pass.

---

🟡 Medium Issues (Needs Decision)

requireOrgAdmin() reimplemented under the same name as an existing shared helper

📍 app/api/admin/email-domain/route.ts:37-44, verify/route.ts:33-42

lib/members/access.ts already exports a requireOrgAdmin() with the same purpose but with error logging on a failed profile lookup that this PR's reimplementation drops. Fails closed either way — not a security hole — but now two different behaviors share one name across three call sites.

Options: Fix now (swap to the shared helper, small diff) | Track as a broader consolidation issue | Skip (consistent with majority of existing routes). Recommended: fix now.

Other MEDIUM issues

load() overwrites a good row with null on read failure

📍 app/admin/settings/email/page.tsx:69-81

Falls through to setRow(data ?? null) even on error, clearing an already-displayed domain and showing "Claim domain" instead of the status card. Sibling pattern (app/admin/families/page.tsx) returns early on error. Recommended: fix now — one-line change.

Unguarded fetch() calls leave the UI stuck "busy" on network failure

📍 app/admin/settings/email/page.tsx:87-154 (handleClaim/handleVerify/handleRemove)

No try/catch around the fetch itself — a thrown network error skips setBusy(false), leaving the button stuck disabled. Mirrors an existing gap in app/admin/families/page.tsx. Recommended: fix now — wrap in try/finally.

Asymmetric error-recovery path (post-create DB update failure) is untested and undocumented

📍 app/api/admin/email-domain/route.ts:509-532

Same region as HIGH #1 — pin the current (or fixed) behavior with a test so the gap is documented rather than silent. Recommended: fix now, bundled with HIGH #1 and #3.

---

🟢 Low Issues

View 5 low-priority suggestions

Issue	Location	Suggestion
Grant-matrix pgTAP checks catalog privileges, not a live UPDATE attempt	supabase/tests/org_email_domains_suite.sql:1069-1083	Add a live-statement block mirroring the existing admin_self_verify_err pattern
owner_b fixture created but never used	supabase/tests/org_email_domains_suite.sql:903,911,926	Mirror the org-A assertion block for org B; low risk given the symmetric policy
Forward reference to org_domains table that doesn't exist yet, uncited	supabase/migrations/20260819000000_org_email_domains.sql:74	Add (docs/plans/phase-5-domains-email.md §6, not yet built)
Org-anchor comment overclaims which callers it covers	app/api/admin/email-domain/route.ts:48-50	Broaden to "every query below in this file" — DELETE uses the same anchor without a service-role client
§10.1/§10.2 status-vocabulary listings also stale	docs/plans/phase-5-domains-email.md	Bundle with HIGH #5's fix

---

✅ What's Good

• Tenancy/RLS layer independently verified by three agents: restrictive isolation policy, column-level GRANT matrix, and tenant-root FK shape all check out against CLAUDE.md's rules.
• Both service-role routes correctly anchor org_id on the caller's own RLS-scoped profile; service-role inventory updated correctly in the same PR.
• Claim route's insert-failure rollback is genuinely well-built and logs if the rollback itself fails.
• pgTAP suite (39 assertions) asserts real row counts and catalog privileges rather than "no error" — avoids this codebase's known "zero-row writes report success" failure mode.
• Comment quality is high throughout; every checkable claim was independently verified against the actual GRANT/CHECK/RLS statements and the installed Resend SDK types.
• The widened status CHECK (Deviation 1) is sound and independently re-verified against the installed resend@6.20.0 SDK.

---

📋 Suggested Follow-up Issues

Issue Title	Priority	Related Finding
Consolidate the three inline requireOrgAdmin()-shaped gates onto lib/members/access.ts's shared helper	P2	MEDIUM — broader than this PR
Wrap admin-page fetch() handlers in try/finally to prevent stuck-busy state	P3	MEDIUM — same gap exists in app/admin/families/page.tsx today

---

Next Steps

1. 🟠 Address the 5 HIGH issues — all are scoped, mechanical changes with recommended code above.
2. 🟡 Review the 4 MEDIUM issues — all recommended "fix now" given low effort relative to risk.
3. 🟢 Consider the 5 LOW issues for this PR or a fast follow-up.
4. This PR is currently a draft with CI still in-progress at review time — confirm all checks resolve green before requesting merge review.

---

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: /Users/cody/.archon/workspaces/cwaits6/two42/artifacts/runs/d29e424939880f07f32879a9d1e4375f/review/

[cwaits6]

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-feat-cwa-70-org-email-domains
Philosophy: Fix everything unless clearly a new concern

---

Fixes Applied (14 total)

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	5
🟡 MEDIUM	4
🟢 LOW	5

View all fixes

• ✅ Resend domain orphaned/unlogged on post-create update failure (app/api/admin/email-domain/route.ts) — added symmetric Resend + DB rollback, with rd.id now logged
• ✅ Missing top-level try/catch around Resend SDK calls (route.ts, verify/route.ts) — wrapped both handlers, matching the codebase's existing convention
• ✅ Claim route's rollback path had zero test coverage — new route.test.ts pins it (mocked service client, requireOrgAdmin, Resend SDK)
• ✅ DOMAIN_SHAPE and admin-page pure helpers had zero unit tests — exported and tested both, including boundary/adversarial cases
• ✅ Plan doc's From:-gate spec cited a stale status count (docs/plans/phase-5-domains-email.md §10.1-§10.3) — corrected 4→7 and reworded to gate on equality to 'verified'
• ✅ requireOrgAdmin() reimplemented under the same name as the shared helper — both routes now import lib/members/access.ts's version
• ✅ load() clobbered a good row with null on read failure (page.tsx) — now returns early, matching the sibling pattern
• ✅ Unguarded fetch() left the UI stuck "busy" on network failure — wrapped all three handlers in try/finally
• ✅ Asymmetric error-recovery path untested — covered by the same test harness as the rollback fix
• ✅ pgTAP grant-matrix checked catalog privileges, not a live UPDATE — added a live-statement UPDATE-denial assertion
• ✅ owner_b fixture created but never used — added an org-B own-access isolation block
• ✅ Uncited forward reference to org_domains — added the plan-doc citation
• ✅ Org-anchor comment overclaimed its callers — resolved by removing the local requireOrgAdmin() it was attached to
• ✅ §10.1/§10.2 stale 5-value status list — updated in the same pass as the §10.3 fix

---

Tests Added

• app/api/admin/email-domain/route.test.ts — 20 tests: rollback on Resend create/update failure, cleanup-failure logging, thrown-exception rollback, duplicate-claim 409, malformed-domain 400, DOMAIN_SHAPE boundary cases
• app/admin/settings/email/page.test.ts — 15 tests: toDnsRecords, statusVariant (incl. unrecognized-status fallback), statusLabel

---

Skipped (0)

(none — all findings addressed)

---

Suggested Follow-up Issues

1. Consolidate remaining inline org-admin gates onto the shared helper — this PR's two routes are done, but most other admin routes still inline the same pattern
2. Wrap app/admin/families/page.tsx's fetch handlers in try/finally — same stuck-busy gap this PR fixed in the new page, still open elsewhere

---

Validation

✅ Type check | ✅ Lint | ✅ Tests (263 passed, up from 228) | ✅ guard:tenancy | ✅ npm run build | ✅ pgTAP org_email_domains_suite.sql (42/42, up from 39/39) | ✅ pgTAP tenancy_leak_suite.sql | ✅ pgTAP schema_tenancy_lint.sql (35/35)

---

Self-fix by Archon · aggressive mode · fixes pushed to archon/task-feat-cwa-70-org-email-domains