fix(api): resolve transaction-rollback + GoogleEventId overflow prod errors

[thomasluizon]

Fixes three live Sentry production errors reported in the #alerts channel. Two share a single root-cause antipattern; one is an undersized column.

Bug 1 — POST /api/Auth/refresh 500s (ORBIT-API-P / -Q / -R)

Under concurrent refresh (parallel mobile clients), TryAcquireCoreAsync's save hits a Serializable-isolation conflict (Postgres 40001/23505). Postgres aborts the transaction server-side, so the explicit transaction.RollbackAsync() in the catch calls CheckReady() and throws InvalidOperationException: This NpgsqlTransaction has completed — which masks the real retryable conflict. Since that IOE isn't retryable, the rate-limit check fails → 500.

• DistributedRateLimitService: drop the manual RollbackAsync (the await using transaction disposal rolls back safely, even on an aborted tx), so the original retryable exception surfaces to the retry loop.
• Also fixed a second defect: the retry filter's attempt < maxAttempts - 1 guard left a final-attempt conflict uncaught and made the terminal "failed after retrying" throw unreachable dead code. Removed the guard so it behaves as documented.

Hardening — the same antipattern in the shared helper

UnitOfWork.ExecuteInTransactionAsync had the identical bug: its RollbackAsync threw on an aborted tx and thereby skipped the ChangeTracker.Clear() on the next line, leaking a poisoned entity onto the shared per-request DbContext. That is exactly what produced the misattributed 22001 below (surfaced at a later SaveChanges). Same fix — hardens all 7 callers.

Bug 2 — POST /api/Chat/stream 22001 (ORBIT-API-N)

value too long for type character varying(256). Confirmed by exhaustive elimination to be a GoogleEventId column: varchar(256) is too small for Google Calendar event ids (documented up to 1024; recurring-instance / imported ids exceed 256), sourced unguarded at GoogleCalendarEventFetcher.cs. Widened Habit.GoogleEventId and GoogleCalendarSyncSuggestion.GoogleEventId to varchar(1024).

⚠️ Deploy note: includes migration 20260708162519_WidenGoogleEventIdColumnsTo1024 (two AlterColumn 256→1024). Additive / backward-compatible (only relaxes a constraint), no index rebuild — safe to auto-deploy on merge.

Tests

Unit tests added for the retry/exhaustion behavior, the UnitOfWork clear-and-rethrow contract, and the widened column lengths. dotnet build 0 errors; Orbit.Infrastructure.Tests 1391 passed. (Coverage note: the exact Npgsql server-side-abort path needs a live Postgres — SQLite/InMemory can't reproduce it — so these tests lock the behavior contract rather than the driver interaction.)

Follow-up (separate)

The RollbackAsync-inside-await using antipattern appeared in 2 places — a Roslyn analyzer to ban it will be filed as its own issue.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #302 — fix(api): resolve transaction-rollback + GoogleEventId overflow prod errors

Scope: PR #302 in thomasluizon/orbit-api (fix/sentry-prod-errors → main), reviewed at merge commit f545a8a
Recommendation: APPROVE

Summary

Fixes three live Sentry production errors: a masked-exception bug in the rollback path of two transactional-retry helpers (UnitOfWork.ExecuteInTransactionAsync and DistributedRateLimitService.ExecuteRelationalAttemptAsync), a dead-code retry-filter guard in the same rate limiter, and an undersized GoogleEventId varchar column. All three fixes are root-cause, narrowly scoped, and backed by new unit tests. No security, contract, or backward-compatibility issues found.

Findings

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	0
Low / Info	1

Critical

None

High

None

Medium

None

Low / Info

[Info] Domain entity has no length guard on GoogleEventId, mirroring a pre-existing gap
· dimension: Backend hard rules (#13, validation)
· location: src/Orbit.Domain/Entities/Habit.cs:85 (SetGoogleEventId), src/Orbit.Domain/Entities/GoogleCalendarSyncSuggestion.cs
· issue: Neither the domain factory/mutator nor any FluentValidation validator enforces a max length on GoogleEventId; the only enforcement is the DB column width (now 1024). This gap predates this PR — no validator regressed by this diff. The value is sourced from Google Calendar's API, not raw user input, so risk is low.
· risk: A future Google-API response with an id longer than 1024 chars would still fail at SaveChanges with the same 22001-class error, just at a higher ceiling.
· fix: Optional follow-up — add a domain guard capping at 1024 so an oversized id fails fast with a clear domain exception instead of a DB error. Not required to merge.
· reference: orbit-api CLAUDE.md "Validation" hard rule

Subagents

Agent	Verdict
security-reviewer	PASS — no findings. Transaction-disposal rollback path verified safe via await using semantics (no early-return/non-disposal code path exists); retry-guard removal is strictly bounded by the outer for (attempt < maxAttempts) loop, not a bypass; GoogleEventId widen is a symmetric, non-truncating, additive migration on a Google-sourced (not raw user-input) field.
contract-aligner	N/A — diff touches no DTO, Controller route, or packages/shared type; purely internal persistence/service-layer fix.

Validation

Check	Result
Build (dotnet)	N/A — skipped per CI adaptation; this PR runs Build / Unit Tests / SonarCloud as separate required CI checks
Tests (dotnet)	N/A — skipped for the same reason; PR body states dotnet build 0 errors, Orbit.Infrastructure.Tests 1391 passed (author-reported, covered by the separate CI check)

What's good

• Correctly diagnoses and fixes the real root cause: NpgsqlTransaction.RollbackAsync() throws InvalidOperationException: This NpgsqlTransaction has completed when Postgres has already aborted the transaction server-side (Serializable-isolation conflict), masking the original retryable exception. Relying on await using disposal (which rolls back safely on an already-ended transaction) is the correct fix, applied consistently in both places it appeared.
• The attempt < maxAttempts - 1 guard removal is verified correct: the outer for (attempt = 0; attempt < maxAttempts; attempt++) loop still bounds total attempts at 3 — this is a straight fix of unreachable dead code (the post-loop throw new InvalidOperationException("...failed after retrying...") was previously unreachable), not a behavior change in attempt count. Confirmed by the new ExhaustsRetries_ThrowsFailedAfterRetrying test asserting exactly 3 SaveChangesAttempts.
• "Hardens all 7 callers" claim in the PR body is accurate — verified ExecuteInTransactionAsync has exactly 7 call sites (ResetAccountCommand, ApplyOnboardingCommand, RunCalendarAutoSyncCommand, BulkCreateHabitsCommand, BulkDeleteHabitsCommand, BulkSkipHabitsCommand, BulkLogHabitsCommand).
• Migration (20260708162519_WidenGoogleEventIdColumnsTo1024) is a clean, symmetric, additive AlterColumn (256→1024) on both affected tables, matching the Fluent API config changes in OrbitDbContext.cs and the model snapshot — no data-loss risk on Up, correctly flagged in the PR description as safe to auto-deploy.
• No hardcoded 256-length validator existed elsewhere to fall out of sync with the new 1024 column width (verified via grep) — the fix is complete.
• Honest test-coverage caveat in the PR body: the exact Npgsql server-side-abort path needs live Postgres and can't be reproduced in SQLite/InMemory, so the new tests lock the surrounding behavior contract (ChangeTracker.Clear() + original-exception rethrow, retry/exhaustion counts) rather than claiming to reproduce the driver-level interaction. Accurately scoped, not oversold.

Recommendation

Safe to merge. No Critical/High findings; the one Info-level note is optional follow-up, not a blocker.