Sprint 9 P2 cleanup: resolve 6 deferred Codex follow-ups in one bundle

CLAUDE.md

@@ -128,13 +128,20 @@ This repository uses local Codex review through `openai/codex-plugin-cc`.
 Before shipping or merging a PR, run Codex review from Claude Code:
 
 ```
-/codex:review --base main
+git fetch origin main
+/codex:review --base origin/main
 ```
 
+Use the remote ref (`origin/main`), not local `main` — if your checkout
+hasn't fetched recently, reviewing against local `main` runs the
+comparison against a stale base and the verdict won't reflect the diff
+GitHub will actually merge.
+
 For larger changes, prefer background review:
 
 ```
-/codex:review --base main --background
+git fetch origin main
+/codex:review --base origin/main --background
 /codex:status
 /codex:result
 ```

api/core/config.py

@@ -36,7 +36,12 @@ class Settings(BaseSettings):
     # Email Service (General)
     EMAIL_FROM: str = "noreply@signupflow.io"
     EMAIL_FROM_NAME: str = "SignUpFlow"
-    EMAIL_ENABLED: bool = True
+    # Default off, matching EmailService.__init__ (api/services/email_service.py).
+    # Both gates must agree — otherwise notification_service._should_queue_email()
+    # reads True from Settings and queues Celery jobs that the worker silently
+    # no-ops because EmailService.enabled is False. See docs/saas/SMOKE_TESTING_EMAIL.md
+    # for how to flip this on (env: EMAIL_ENABLED=true).
+    EMAIL_ENABLED: bool = False
     SMS_ENABLED: bool = False
 
     # Mailtrap (Development Email Testing)

api/routers/password_reset.py

@@ -131,6 +131,26 @@ def request_password_reset(
     if not person:
         return generic_response
 
+    # Re-acquire the Person row with ``FOR UPDATE`` immediately before the
+    # critical section (invalidate prior tokens + insert new + commit).
+    # The initial Person lookup above can't hold the lock, because
+    # ``log_audit_event`` commits the audit row in between — which would
+    # release any row lock taken on the original query. Re-locking here
+    # scopes the lock tightly to the rotation block: two concurrent
+    # /forgot-password requests for the same person serialize on this
+    # SELECT, then run the UPDATE/INSERT/commit one at a time, so the
+    # freshness contract in docs/features/password-reset.md Scenario 6
+    # holds. SQLite ignores FOR UPDATE (its global lock already serializes
+    # writers); PostgreSQL enforces a real row lock under READ COMMITTED.
+    # Scope by org_id too — repo-wide tenancy contract is "every Person
+    # query filters by org_id". Filtering by primary key alone is
+    # technically sufficient here (Person.id is globally unique), but the
+    # convention exists so reviewers can grep for missing org filters in
+    # new code paths.
+    db.query(Person).filter(
+        Person.id == person.id, Person.org_id == person.org_id
+    ).with_for_update().one()
+
     now = utcnow()
 
     # Invalidate any prior unused reset tokens for this person before

docs/saas/SMOKE_TESTING_EMAIL.md

@@ -44,6 +44,20 @@ Mailtrap captures every send to a virtual inbox you can view in the
 browser. Nothing leaves their network — safe to use without any domain
 authentication.
 
+> **Shell prelude (important):** before running the smoke, ensure no
+> stale `SENDGRID_API_KEY` is exported in your shell:
+>
+> ```bash
+> unset SENDGRID_API_KEY
+> ```
+>
+> The smoke script reloads `.env` with `override=True`, so a blank
+> `SENDGRID_API_KEY=` line in `.env` will clear it for the run; but an
+> exported value in the shell can still leak in via subprocess
+> inheritance for the Python interpreter that imports SendGrid. The
+> unset is a belt-and-braces guarantee that you're testing the SMTP
+> path, not accidentally hitting live SendGrid.
+
 ### 1. Get credentials
 
 1. Sign up / log in at <https://mailtrap.io/>.

mobile/lib/api/api_client.dart

@@ -17,10 +17,16 @@ const String defaultApiBaseUrl = String.fromEnvironment(
   defaultValue: 'http://localhost:8000',
 );
 
+/// Outcome of one /auth/refresh attempt. The interceptor must distinguish
+/// "refresh failed" (wipe storage) from "refresh succeeded but storage was
+/// mutated mid-flight" (leave storage alone) — otherwise a signOut / fresh
+/// login that races a stale refresh response gets clobbered.
+enum _RefreshOutcome { success, failure, stale }
+
 /// Pump a single concurrent /auth/refresh attempt. If two requests hit
 /// 401 at the same time, only one fires the refresh; the others await
 /// the same `Future` and replay against the new token.
-Completer<bool>? _refreshInFlight;
+Completer<_RefreshOutcome>? _refreshInFlight;
 
 /// dio configured with the API base URL + a token-refresh interceptor.
 final dioProvider = Provider<Dio>((ref) {
@@ -52,14 +58,21 @@ final dioProvider = Provider<Dio>((ref) {
           return;
         }
 
-        final refreshed = await _attemptRefresh(dio, storage);
-        if (!refreshed) {
+        final outcome = await _attemptRefresh(dio, storage);
+        if (outcome == _RefreshOutcome.failure) {
           // No refresh token, refresh failed, or 401 from /auth/refresh
           // itself — wipe both tokens and let the router bounce to /login.
           await storage.clearAll();
           handler.next(e);
           return;
         }
+        if (outcome == _RefreshOutcome.stale) {
+          // Storage was mutated (signOut / fresh login) while /auth/refresh
+          // was in flight. Don't replay with the dropped tokens, but also
+          // don't clear the user's current session — leave storage alone.
+          handler.next(e);
+          return;
+        }
 
         // Replay the original request with the new access token.
         try {
@@ -81,22 +94,31 @@ final dioProvider = Provider<Dio>((ref) {
 });
 
 /// Fires `/auth/refresh` against the stored refresh token. Coalesces
-/// concurrent calls so only one round-trip happens at a time. Returns
-/// true on success (new tokens persisted), false otherwise.
-Future<bool> _attemptRefresh(Dio dio, SecureTokenStorage storage) async {
+/// concurrent calls so only one round-trip happens at a time. The
+/// outcome tells the caller whether to (success) replay the original
+/// request, (failure) clear storage and propagate 401, or (stale) just
+/// propagate 401 without touching storage.
+Future<_RefreshOutcome> _attemptRefresh(Dio dio, SecureTokenStorage storage) async {
   // Coalesce concurrent refreshes.
   final inFlight = _refreshInFlight;
   if (inFlight != null) {
     return inFlight.future;
   }
-  final completer = Completer<bool>();
+  final completer = Completer<_RefreshOutcome>();
   _refreshInFlight = completer;
 
+  // Snapshot the storage generation at the start. Any storage mutation
+  // (signOut, fresh login, another refresh landing first) bumps it;
+  // we check before persisting so an in-flight stale response can't
+  // overwrite whatever's currently in storage. Hoisted out of the try
+  // block so the DioException catch can also see it (Dart scoping).
+  final genAtStart = storage.sessionGeneration;
+
   try {
     final refreshToken = await storage.readRefreshToken();
     if (refreshToken == null || refreshToken.isEmpty) {
-      completer.complete(false);
-      return false;
+      completer.complete(_RefreshOutcome.failure);
+      return _RefreshOutcome.failure;
     }
     final res = await dio.post<dynamic>(
       '/api/v1/auth/refresh',
@@ -113,16 +135,38 @@ Future<bool> _attemptRefresh(Dio dio, SecureTokenStorage storage) async {
     if (body is Map &&
         body['token'] is String &&
         body['refresh_token'] is String) {
+      // Drop the response if anything mutated storage during the
+      // /auth/refresh round-trip — the response is for a session
+      // that's already been replaced or cleared. KNOWN RESIDUAL RACE:
+      // within Dart's single-isolate cooperative model, between the
+      // two awaited writes below another mutator can interleave. We
+      // don't attempt to roll back the first write, because by the
+      // time we detect it the "current" access token may belong to a
+      // fresh login that ran after our write — clearing it would log
+      // that user out. The realistic window is platform-call latency
+      // (microseconds); on the next 401 the interceptor sees whatever
+      // session is actually in storage and rotates from there.
+      if (storage.sessionGeneration != genAtStart) {
+        completer.complete(_RefreshOutcome.stale);
+        return _RefreshOutcome.stale;
+      }
       await storage.writeToken(body['token'] as String);
       await storage.writeRefreshToken(body['refresh_token'] as String);
-      completer.complete(true);
-      return true;
+      completer.complete(_RefreshOutcome.success);
+      return _RefreshOutcome.success;
     }
-    completer.complete(false);
-    return false;
+    completer.complete(_RefreshOutcome.failure);
+    return _RefreshOutcome.failure;
   } on DioException {
-    completer.complete(false);
-    return false;
+    // If the refresh failed AND storage changed since we started, the
+    // failure belongs to a session that's already gone. Don't make the
+    // caller clearAll() — that would wipe the new session.
+    if (storage.sessionGeneration != genAtStart) {
+      completer.complete(_RefreshOutcome.stale);
+      return _RefreshOutcome.stale;
+    }
+    completer.complete(_RefreshOutcome.failure);
+    return _RefreshOutcome.failure;
   } finally {
     _refreshInFlight = null;
   }

mobile/lib/auth/invitation_repository.dart

@@ -88,9 +88,14 @@ class InvitationRepository {
       // Persist the refresh token (Sprint 9 PR 9.4b makes invitation accept
       // mint real JWT pairs like login/signup). Pre-9.4b backends omit the
       // field; treat null/empty as "no refresh — fall back to re-login".
+      // Mirror login_repository / signup_repository: clear any prior refresh
+      // token when the response omits one, so accepting an invitation can't
+      // inherit the previous session's refresh credential.
       final refreshToken = data.refreshToken;
       if (refreshToken != null && refreshToken.isNotEmpty) {
         await _storage.writeRefreshToken(refreshToken);
+      } else {
+        await _storage.clearRefreshToken();
       }
       return AuthState(
         role: LoginRepository.resolveRole(data.roles.toList()),

mobile/lib/auth/secure_token_storage.dart

@@ -18,6 +18,14 @@ abstract class SecureTokenStorage {
   /// Convenience: nukes both tokens. Used on logout and on a 401 that
   /// can't be recovered via /auth/refresh.
   Future<void> clearAll();
+
+  /// Monotonic counter bumped on every write/clear. The refresh
+  /// interceptor captures this at request time and verifies it hasn't
+  /// changed before persisting response tokens. If it has — signOut(),
+  /// a fresh login, or another refresh response landed in between —
+  /// the response is dropped without mutating storage. Closes the
+  /// TOCTOU window in `readRefreshToken() → compare → writeToken()`.
+  int get sessionGeneration;
 }
 
 class _RealStorage implements SecureTokenStorage {
@@ -26,27 +34,44 @@ class _RealStorage implements SecureTokenStorage {
   final _impl = const FlutterSecureStorage(
     iOptions: IOSOptions(accessibility: KeychainAccessibility.first_unlock),
   );
+  int _gen = 0;
+
+  @override
+  int get sessionGeneration => _gen;
 
   @override
   Future<String?> readToken() => _impl.read(key: _accessKey);
 
   @override
-  Future<void> writeToken(String token) => _impl.write(key: _accessKey, value: token);
+  Future<void> writeToken(String token) async {
+    _gen++;
+    await _impl.write(key: _accessKey, value: token);
+  }
 
   @override
-  Future<void> clearToken() => _impl.delete(key: _accessKey);
+  Future<void> clearToken() async {
+    _gen++;
+    await _impl.delete(key: _accessKey);
+  }
 
   @override
   Future<String?> readRefreshToken() => _impl.read(key: _refreshKey);
 
   @override
-  Future<void> writeRefreshToken(String token) => _impl.write(key: _refreshKey, value: token);
+  Future<void> writeRefreshToken(String token) async {
+    _gen++;
+    await _impl.write(key: _refreshKey, value: token);
+  }
 
   @override
-  Future<void> clearRefreshToken() => _impl.delete(key: _refreshKey);
+  Future<void> clearRefreshToken() async {
+    _gen++;
+    await _impl.delete(key: _refreshKey);
+  }
 
   @override
   Future<void> clearAll() async {
+    _gen++;
     await _impl.delete(key: _accessKey);
     await _impl.delete(key: _refreshKey);
   }
@@ -56,27 +81,44 @@ class _RealStorage implements SecureTokenStorage {
 class InMemoryTokenStorage implements SecureTokenStorage {
   String? _token;
   String? _refresh;
+  int _gen = 0;
+
+  @override
+  int get sessionGeneration => _gen;
 
   @override
   Future<String?> readToken() async => _token;
 
   @override
-  Future<void> writeToken(String token) async => _token = token;
+  Future<void> writeToken(String token) async {
+    _gen++;
+    _token = token;
+  }
 
   @override
-  Future<void> clearToken() async => _token = null;
+  Future<void> clearToken() async {
+    _gen++;
+    _token = null;
+  }
 
   @override
   Future<String?> readRefreshToken() async => _refresh;
 
   @override
-  Future<void> writeRefreshToken(String token) async => _refresh = token;
+  Future<void> writeRefreshToken(String token) async {
+    _gen++;
+    _refresh = token;
+  }
 
   @override
-  Future<void> clearRefreshToken() async => _refresh = null;
+  Future<void> clearRefreshToken() async {
+    _gen++;
+    _refresh = null;
+  }
 
   @override
   Future<void> clearAll() async {
+    _gen++;
     _token = null;
     _refresh = null;
   }