Track and resend organization invitation emails

[PierreLeGuen]

Summary

• add admin endpoints to list invitation email deliveries and resend pending invitations
• expose delivery and invitation status fields in API responses
• add repository and service support plus coverage for list and resend flows

Testing

• cargo fmt --check
• cargo check -p services -p database -p api
• cargo test -p services resend_invitation_email --lib
• cargo test -p api --test e2e_all admin_invitation_email_deliveries -- --nocapture (blocked locally: Postgres connection refused)

Closes #578

[gemini-code-assist]

Code Review

This pull request introduces platform admin endpoints to list organization invitation email deliveries and resend invitation emails, along with their corresponding database queries, service logic, API models, and integration tests. The review feedback highlights a critical PostgreSQL type mismatch issue in the database repository where custom enum columns are compared directly to text parameters in both the list and count queries. Additionally, it suggests an improvement to update the invitation status to Expired in the database when a resend is attempted on an expired invitation.

[gemini-code-assist]

In PostgreSQL, comparing a custom enum column directly to a parameter explicitly cast to text (e.g., $3::text or $4::text) will result in a type mismatch error (operator does not exist: invitation_email_status = text) because PostgreSQL does not implicitly coerce text to custom enum types.

To prevent runtime database errors, cast the enum columns to text in the comparison.

Suggested change

	AND ($3::text IS NULL OR i.email_status = $3)
	AND ($4::text IS NULL OR i.status = $4)
	AND ($3::text IS NULL OR i.email_status::text = $3)
	AND ($4::text IS NULL OR i.status::text = $4)

[gemini-code-assist]

Apply the same enum-to-text casting in the count query to avoid type mismatch errors when filtering by email_status or status.

Suggested change

	AND ($3::text IS NULL OR i.email_status = $3)
	AND ($4::text IS NULL OR i.status = $4)
	AND ($3::text IS NULL OR i.email_status::text = $3)
	AND ($4::text IS NULL OR i.status::text = $4)

[gemini-code-assist]

When a resend attempt is made on an expired invitation, its status in the database should be updated to Expired to maintain data consistency. This mirrors the behavior in accept_invitation_impl where expired invitations are explicitly marked as Expired in the database.

Suggested change

	if invitation.expires_at < chrono::Utc::now() {
	return Err(OrganizationError::InvalidParams(
	"Invitation has expired".to_string(),
	));
	}
	if invitation.expires_at < chrono::Utc::now() {
	let _ = self
	.invitation_repository
	.update_status(invitation_id, ports::InvitationStatus::Expired)
	.await;
	return Err(OrganizationError::InvalidParams(
	"Invitation has expired".to_string(),
	));
	}

[claude]

PR #674 Review — Track and resend organization invitation emails

Reviewed the diff, repository SQL, service/route flow, and tests. Overall the change is well-structured (port/adapter separation, sanitized error storage, admin-only routes, and solid test coverage — auth gate, expired/non-pending guards, ordering, filter combinations). A few items worth addressing:

⚠️ Issues

1. resend_invitation_email_impl silently swallows a deleted invitation on reload — crates/services/src/organization/mod.rs:1246-1255
```rust
let updated_invitation = self
.invitation_repository
.get_by_id(invitation_id)
.await
.map_err(...)?
.unwrap_or(invitation);
```
If the invitation is deleted between the send and the reload, Ok(None) falls back to the pre-send invitation, so email_status / email_sent_at / email_last_error returned to the admin will be the stale pre-send values even though the email was attempted. Either propagate a not-found error, or (preferred) construct the response from the in-memory record_email_* outcome rather than re-reading.

2. Count query in list_email_deliveries does unnecessary joins — crates/database/src/repositories/organization_invitation.rs:846-865
```sql
SELECT COUNT(*) AS total
FROM organization_invitations i
JOIN organizations o ON o.id = i.organization_id
LEFT JOIN users u ON u.id = i.invited_by_user_id
WHERE ...
```
Nothing in the WHERE clause references o or u, so the joins are pure overhead on every list call. Drop them — SELECT COUNT(*) FROM organization_invitations i WHERE ... is equivalent and cheaper. (Side note: the data query uses JOIN organizations, so a row with a missing org is silently filtered there but counted here — that divergence is worth flagging either way.)

3. No index supports the list ordering / date range — follow-up migration
Query is ORDER BY i.created_at DESC, i.id DESC with optional created_at range. Existing indexes are on organization_id, email, status, token, and (partial) expires_at. As the table grows, unfiltered admin listings will sort the whole table. Consider CREATE INDEX idx_org_invitations_created_at ON organization_invitations(created_at DESC) (or composite (organization_id, created_at DESC)) in a follow-up.

4. recipient_email ILIKE filter does not escape % / _ — crates/database/src/repositories/organization_invitation.rs:815
```sql
AND ($2::text IS NULL OR i.email ILIKE '%' || $2 || '%')
```
Admin-only, so not exploitable, but % / _ in the input match more than intended. Either document substring semantics or escape LIKE wildcards. Low severity.

5. Skipped resend outcome is ambiguous to the admin
When invitations_url is None, send_invitation_email returns (false, None) and records Skipped. The admin sees success: false, email_sent: false, email_status: Skipped, error: None with no hint that the cause is server misconfiguration. Return an explicit error: Some("invitation URL not configured") (or a 503) so this is distinguishable from a Resend API failure.

✓ Looks good

• Stored error is sanitized_message(), so email_last_error exposure to admin is safe (per CLAUDE.md privacy rules).
• tracing::info! logs only IDs and the email_status enum — no recipient email or token leaked.
• Tokens are never exposed by the new admin response shapes.
• Pending-only and expiry guards before send are correct, and tests cover both.
• Parameterized SQL throughout — no injection risk.
• validate_limit_offset enforces 1..=1000 bounds at the route boundary.

⚠️ Issues found — non-blocking, but please address #1 and #2 before merge; #3–5 can be follow-ups.

[PierreLeGuen]

Addressed review feedback:

• Cast invitation enum columns to text in list/count filters.
• Mark expired invitations as expired during resend attempts before returning the validation error.
• Kept the admin route builder under the lint argument limit by bundling route dependencies.
• Removed unnecessary joins from the delivery count query.
• Avoided stale resend responses by using the recorded invitation result when available and returning not found if a fallback reload cannot find the invitation.
• Return an explicit error when invitation email delivery is skipped because the invitation URL is not configured.

Local checks:

• cargo fmt --all -- --check
• cargo clippy --all-targets --all-features -- -D warnings
• cargo check -p services -p database -p api
• cargo test -p services resend_invitation_email --lib

[Evrard-Nil]

Pulled the branch, reviewed end-to-end, and ran the test suite locally.

Build: cargo check -p services -p api -p database clean.
Tests: All 11 invitation tests pass (5 new resend_invitation_email_* + 5 existing create_invitations_* + 1 email rendering). No regressions.
Scope check: Diff vs. correct base (837c4a5) is ~1210 additions across 11 files — the main..pr-674 apparent removal of thought_signature is just an artifact of the PR being branched before #649 merged, not a real revert. Worth rebasing to get rid of that confusion.

Overall the change is well-structured. Refactoring send_invitation_email from (bool, Option<String>) to a InvitationEmailAttempt struct that threads back the updated OrganizationInvitation is a clean improvement and the resend path reuses it cleanly. Single-query list_email_deliveries with the JOIN — good (avoided the N+1 pattern that bit cloud-api#662). Admin-only gating via AdminUser extension confirmed. Tokens are NOT exposed in the admin response — verified the response model has no token field.

One important thing worth surfacing as a real concern:

🔒 email_last_error privacy surface. sanitize_error() (crates/services/src/email.rs:40) only collapses whitespace and truncates to 1000 chars — it is not a PII scrubber. If Resend ever returns something like \"Invalid recipient: foo@bar.com\" or \"Domain blocked: example.com\", that goes straight into the DB column email_last_error and surfaces via GET /v1/admin/invitation-email-deliveries. Per the privacy rules in CLAUDE.md ("Never log request/response bodies — Even at debug level, unless you're 100% certain they don't contain customer data"), this is a soft violation. This isn't a log strictly, but it's persistent and externally visible. Either:

• Tighten sanitize_error to strip email-looking patterns and provider-side identifiers, OR
• Document explicitly that this is admin-only inside the TEE perimeter and that acceptable downstream consumers are trusted (and update CLAUDE.md if so).

Non-blocking observations:

1. Subtle behavior change to existing create_invitations response. When invitations_url is unset, email_error was None; now it's Some(\"Invitation URL is not configured\"). The unit test at crates/services/src/organization/mod.rs:2254 was updated to match, but external clients that parsed email_error == None as "success" will see a new string. Worth a one-liner in the PR description / changelog so consumers know.

2. No anti-abuse on /resend. An admin (or compromised admin session) can hit POST …/{id}/resend in a tight loop. Each call performs a network roundtrip to Resend and writes a DB row. Resend's external rate limit will eventually catch this, but we'd churn email_last_error rows. Probably fine because admin-only, but a per-invitation cooldown (e.g. "can't resend within 30s of the last attempt") would be defensive.

3. TOCTOU between status read and send. resend_invitation_email_impl reads status=Pending, then calls the network-bound send_invitation_email, then writes results. Between the read and the send, the invitee may have accepted/declined — race is benign (one extra email goes out for an already-accepted invite, status stays Accepted because record_email_* doesn't change status). Worth a code comment noting that's intentional.

4. List pagination consistency. list_email_deliveries issues the rows query and the count query via separate pool acquisitions. Under concurrent writes, total and deliveries.len() can briefly disagree. Acceptable for admin observability; calling it out so it's not a surprise later.

5. No composite index for the sort. ORDER BY i.created_at DESC, i.id DESC plus org_id filter would benefit from a composite (organization_id, created_at DESC, id DESC) index when invitation rows grow. Not needed today.

6. Email substring search is unindexable. email ILIKE '%' || $2 || '%' prevents the idx_org_invitations_email index from being used; full filter scan only. Acceptable for admin audit.

7. Resend endpoint lacks an e2e test. The list endpoint has admin_invitation_email_deliveries.rs, which is great. The resend endpoint relies on unit tests only. Adding a simple e2e (insert invitation fixture → POST resend → assert email_status changed) would round it out — author's own validation note says local Postgres was refused, so this would also unblock that gate.

LGTM. The two critical things author needs to confirm/address are (a) the email_last_error PII surface (CLAUDE.md privacy rule), and (b) the create_invitations response behavior change in changelog/migration notes. Everything else is polish. Approving.