Conversation
Tyjfre-j
marked this pull request as draft
July 20, 2026 12:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the original Python-side session cap enforcement with a database-native eviction strategy using
FOR UPDATE SKIP LOCKED, eliminating race conditions under concurrent logins without adding application-level locking.What changed
SQL queries (
db/queries/session.sql)Added:
EvictOverflowSessions—DELETE ... RETURNINGwithFOR UPDATE SKIP LOCKEDsubquery. Computes overflow internally viaCOUNT(*), sorts by(last_active, created_at), and atomically evicts the oldest sessions.LockUserSessions— advisorySELECT ... FOR UPDATEto serialize per-user session mutations.Removed:
EvictOldestSessions— replaced byEvictOverflowSessions(different parameter contract: passessession_limitinstead of pre-computedlimit).Service layer (
app/service/users.py)_create_mobile_sessionflow updated:lock_user_sessions— serialize per-user mutations (acquires row lock on all user sessions)_ensure_device_for_login— get/create device rowupsert_session— create/replace session for this deviceevict_overflow_sessions— evict oldest if over cap (yields evicted session IDs)Replaces Python-side
list_sessions_by_user+ in-memory sort +delete_session_by_idwith atomic SQL eviction.Test updates
Updated fixtures/fakes across 4 test files to match new query signatures:
tests/unit/test_auth_service.py—TestSessionLimitupdated forevict_overflow_sessions+lock_user_sessions; addedtest_multiple_new_devices_at_cap_evict_exact_overflowtests/unit/test_mobile_auth_intent_validation.py— statefulFakeSessionQueriernow computes overflow internally fromsession_limittests/unit/test_mobile_auth_email_logging.py— added missinglock_user_sessions+evict_overflow_sessionstests/unit/test_auth_email_otp.py— configuredevict_overflow_sessionson bareAsyncMock(async generator side-effect)Added integration test:
test_concurrent_new_device_logins_settle_at_cap_real_db— stress test using separate DB connections per concurrent task; validates final session count ==SESSION_LIMITunder true Postgres concurrencyWhy SKIP LOCKED
Without it, two concurrent logins from the same user at cap could both read "3 sessions," both decide to evict, and both create a new session → 4 sessions briefly exist.
FOR UPDATE SKIP LOCKEDmakes the subquery rows invisible to competing transactions, so evictions serialize cleanly at the database level.The advisory lock (
lock_user_sessions) ensures the entire device-lookup → upsert → evict sequence is serialized per user, preventing interleaving betweenupsert_sessionandevict_overflow_sessions.