feat(db): accounts on native Postgres — #251

[ryanbarlow97]

Accounts move to native Postgres (#251): users, cookie sessions, Discord OAuth states, preview sign-in tickets and the staff audit log are now read and written through src.database with %s placeholders, RETURNING, ON CONFLICT and the new column types. Every accounts path from the foundation PR's table has its locking recipe, and each recipe has a test that races two connections against the real Postgres. Other areas stay on SQLite; where they meet accounts there is a temporary bridge marked PG-MIGRATION.

Previews of the postgres branch cannot sign in until the host-plumbing PR lands. ps-preview.sh gives a preview no Postgres and no DATABASE_URL, and accounts now live only in Postgres. Such a preview, or a local run without Postgres, serves signed-out pages normally: nothing touches the database until a session token exists. Sign-in (/auth/discord/start) and any request carrying a session cookie answer 503 accounts_unavailable instead of a 500. So do the staff writes (role change, session revocation, server actions, view-as start) and restoring an admin from the staff cookie alone (GET /account, POST /auth/view-as/stop). Each checks for the database before opening a transaction, so nothing is written. A preview's callback checks before spending dev's ticket. Signing out still clears cookies, and the view-as guard lets writes through, because no view can exist without the database. The preview data-copy scrub of user_sessions and discord_oauth_states now hits stale SQLite tables; it carries a PG-MIGRATION: host plumbing mark.

Files converted

• backend/src/auth/users.py: sign-in, sessions, OAuth states, session_user_in(conn, token, lock=False), drop_view_leftovers, PLACEHOLDERS_LOCK.
• backend/src/auth/audit.py: record writes detail_json as jsonb; record_refusal uses its own Postgres transaction.
• backend/src/auth/admin.py: role changes, session revocation, staff list and lookup.
• backend/src/auth/admin_cli.py: _set_root on Postgres; new --database-url (default DATABASE_URL).
• backend/src/auth/view_as.py: start, stop, end, end_lapsed, view_still_open, refuses_write.
• backend/src/auth/preview_sign_in.py: ticket issue and redeem.
• backend/src/auth/players.py: the website accounts in the Players directory, and its two audit writes.
• backend/src/api/admin_routes.py: _server_write.
• backend/src/api/database_routes.py: the studio's per-database query counts read detail_json ->> 'database'.

Bridged where another area meets accounts (each marked, see below): auth/microsoft_link.py (still_valid), skins/discord_link.py (_cancel_microsoft_attempts), luckperms/views.py (_account) and luckperms/changes.py (_target_account_role, _current_actor). These read users and user_sessions from Postgres, because that is now the only place they are written.

Locking, path by path

Tests are in backend/src/auth/test_locking.py. One transaction holds a lock, either by hand on its own connection or through a Gate that pauses a path just after a chosen statement. The competing path runs on another thread. The test reads pg_stat_activity to see that thread's backend waiting with wait_event_type = 'Lock', and checks which kind of lock: advisory for X3, transactionid or tuple for a row lock. It then lets the holder commit and checks that the waiter acted on the holder's result.

Every path that calls drop_view_leftovers takes X3 first, so two whole calls of such a path are serialised by X3 before any row lock matters. Tests that race two whole calls are therefore listed as X3 tests. To isolate a row-lock or constraint recipe, the test holds the competing row by hand without the advisory lock. I checked that each isolating test fails when its recipe is removed: _lock_accounts dropped, FOR UPDATE or FOR SHARE removed, ON CONFLICT removed, X3 dropped from end, DELETE … RETURNING replaced by a plain read.

Path	Recipe	Test
users.consume_state	B: DELETE … WHERE state_hash = %s RETURNING return_to, expires_at	test_a_discord_state_is_spent_once
users.sign_in	X3 first, then C: INSERT … ON CONFLICT (discord_user_id) DO UPDATE … RETURNING id	C: test_a_first_sign_in_joins_an_account_being_made (an uncommitted insert of the same id held by hand). X3: test_every_leftover_sweep_takes_the_placeholders_lock_first
admin_cli._set_root	X3, then A: target, and for a removal every root, ORDER BY id FOR UPDATE, then count	A: test_root_locks_wait_for_a_panel_demotion (a root's row held by hand). X3: test_two_cli_removals_cannot_leave_no_root
admin._staff_write (role change, revocation)	X3, then A: actor and target users rows ORDER BY id FOR UPDATE, then X1	A: test_a_role_change_waits_for_a_change_to_its_target (the target's row held by hand). X3: test_two_roots_demoting_each_other_run_one_after_the_other
view_as.start	X3, X1, C for the placeholder (ON CONFLICT (discord_user_id) DO NOTHING, re-select), X2 through _open	X3: test_a_view_being_opened_keeps_its_placeholder. C: test_a_placeholder_never_collides_with_a_new_account
view_as.stop, view_as.end	X3 + X2 (_end_where selects the views FOR UPDATE OF s)	X2: test_ending_a_view_locks_its_session_row (no advisory lock). X3: test_two_requests_ending_one_view_wait_for_each_other
view_as.end_lapsed	X3 + X1 + X2	test_every_leftover_sweep_takes_the_placeholders_lock_first
preview_sign_in.redeem	B: DELETE FROM preview_sign_in_tickets t USING users u … RETURNING …	test_a_preview_ticket_is_redeemed_once
admin_routes._server_write	X1: session_user_in(conn, token, lock=True) in the audit entry's transaction	test_a_server_action_and_a_demotion_never_cross
session_user_in(lock=True) itself	FOR SHARE OF s, u; lock=False takes nothing; lock=True with a SQLite connection raises TypeError	test_a_staff_recheck_holds_off_demotion_and_revocation, test_an_ordinary_session_read_takes_no_lock, test_a_lock_cannot_be_asked_of_the_sqlite_bridge
view_as.view_still_open	A: the view row FOR SHARE on a Postgres connection	test_a_view_still_open_check_holds_the_view_until_its_write_commits (ending the view waits for the check's transaction). Its only caller, Skins, still passes SQLite, so live it goes through the bridge, whose false answer test_profile_is_not_opened_for_a_view_that_ended_mid_request covers

X3 is taken as the first statement of every transaction that calls drop_view_leftovers: sign_in, _staff_write, _set_root, view_as.start, stop, end and end_lapsed. Never inside drop_view_leftovers. The foundation PR lists no read-then-write races outside BEGIN IMMEDIATE for this area, so there were none to fix beyond the table.

Output formats

Times are timestamptz now, but nothing a client parses changes. Every time that leaves the accounts code is still YYYY-MM-DDTHH:MM:SSZ, made by users.api_time():

• The session user dict (session_user, session_user_in, guild_state, record_guild_check) carries expires_at, guild_checked_at and created_at in that form. Cookie lifetimes (seconds_left), /account (guild.checked_at, viewing_as.expires_at), the guild check, Profile sessions opened during a view, LuckPerms and diagnostics all keep reading it as before.
• The preview hand-off (POST /auth/preview/redeem) returns guild_checked_at in the Z form, the only one a preview's _checked_at accepts. A preview passes that string back to sign_in, which parses it into a datetime.
• view_as.start returns expires_at in the Z form, which sets the view cookie's lifetime.
• The staff panel (/admin/staff, /admin/accounts/lookup, the account in /admin/players/{uuid}) returns created_at and last_login_at in the Z form.

Stored times now keep microseconds; the Z form drops them, as the old strings did. guild_member and placeholder are booleans, and guild_member comes back as bool, where it used to be 0/1. Every reader already used it as a truth value. admin_audit.detail_json is jsonb and reads back as a dict. Tests pin the formats in test_session_times_keep_the_api_form, test_admin_views_the_site_as_a_player, test_dev_start_hands_a_signed_in_player_to_the_preview and test_profile_shows_the_website_account.

admin.change_role and revoke_sessions answer 404 for a target id past bigint instead of hitting a database error.

PG-MIGRATION marks

Each mark names the area whose conversion removes it. The temporary second paths for a sqlite3.Connection are in audit.record, session_user_in (which refuses lock=True there) and view_still_open. The other marks are reads or deletes on tables that still live in SQLite, done on a separate connection until their area moves.

File	Removed by	What
auth/users.py session_user_in	LuckPerms	a SQLite connection from luckperms/changes.py: the session is read from Postgres on its own connection
auth/audit.py record	LuckPerms	a SQLite connection: the row is written to Postgres in its own transaction, just before the caller's commit
auth/view_as.py view_still_open	Skins	a SQLite connection from start_linked_profile_session: checked on its own Postgres connection, which reopens a narrow window between the check and the caller's commit
auth/users.py drop_view_leftovers	Skins	Profile sessions and codes are deleted in SQLite, against the live view ids from Postgres. This commits its SQLite delete in the middle of the Postgres transaction, so the delete stays if that transaction rolls back. It also takes SQLite's write lock while holding Postgres locks. Accepted until Skins converts
auth/view_as.py refuses_write	Skins	a bearer Profile session and its code are read from SQLite
auth/view_as.py _link_for_player	Linking	discord_links read from SQLite
auth/view_as.py _players_for	Linking	each ended view's player UUID read from SQLite
auth/admin.py _with_minecraft	Linking	Minecraft names for the staff list and lookup read from SQLite
auth/players.py _site_rows	Linking, Characters	links and roster from SQLite, accounts from Postgres, merged in Python
auth/players.py character_picture	Characters	roster read from SQLite
auth/discord_names.py refresh_all	Linking	discord_links read from SQLite
auth/account_overview.py rank	LuckPerms	the LuckPerms mirror read from SQLite
auth/microsoft_link.py still_valid	Linking	the session checked on Postgres, the state on the caller's SQLite connection; reopens a narrow window between the check and the caller's commit
auth/admin_cli.py --db	Scripts	the SQLite file is still needed for Profile sessions and Minecraft names
skins/discord_link.py _cancel_microsoft_attempts	Linking	the user's id looked up in Postgres, only when any Microsoft state is pending
luckperms/views.py _account	LuckPerms	the account's role read from Postgres
luckperms/changes.py _target_account_role	LuckPerms	the target's role read from Postgres
luckperms/changes.py _current_actor	LuckPerms	the actor's role read from Postgres
dbstudio/studio.py sources	Database studio	the "site" source is still province.db; its description now says accounts and the audit log are in Postgres and their tables there are stale
scripts/ps-preview.sh data-copy scrub	host plumbing	deletes user_sessions and discord_oauth_states in the copied SQLite file, which no longer holds them; previews have no DATABASE_URL

Until those areas convert, the bridged reads and writes are not atomic with the SQLite transaction around them. The branch is not deployed, and each area's PR puts the join or the lock back on one connection.

Tests

The accounts tests run on the postgres fixture: test_admin, test_view_as, test_auth_routes, test_preview_sign_in, test_microsoft, test_players and test_staff_tools. Discord links, roster rows, codes and Profile sessions are still seeded in SQLite, through the database fixture. src/auth/testing.py holds small Postgres helpers.

Tests from other areas that sign in through accounts also take postgres: LuckPerms, servers, rail, diagnostics and the database studio. The studio's SQLite tests now query discord_links in place of users, which no longer has rows in province.db. They also allow the extra postgres source that DATABASE_URL adds. No studio test treats an edit to the SQLite accounts tables as meaningful.

test_without_an_accounts_database_pages_work_signed_out_and_sign_in_is_503 covers the no-database case: a signed-out /account, a 503 from sign-in and for a stale cookie, sign-out, and the view-as guard. test_staff_writes_without_an_accounts_database_are_503_and_change_nothing covers the staff writes and the staff-cookie restore: each answers 503, and no role, session or audit row changes. test_preview_callback_without_an_accounts_database_keeps_the_ticket checks that the ticket is never redeemed.

Two SQLite-only tests stay: test_migration_adds_role_to_existing_users and test_migration_adds_acting_user_id cover skins/db._upgrade, which still runs until area 10 removes it.

Full backend suite against pgvector/pgvector:pg17, with TEST_DATABASE_URL set:

• postgres before this PR (b5e161e): 2,432 passed, 4 skipped, 175 subtests passed (14 min 55 s)
• this PR: 2,454 passed, 4 skipped, 175 subtests passed (16 min 47 s). The 22 new tests are the 18 races in test_locking.py, test_session_times_keep_the_api_form and three no-database tests (test_without_an_accounts_database_pages_work_signed_out_and_sign_in_is_503, test_staff_writes_without_an_accounts_database_are_503_and_change_nothing, test_preview_callback_without_an_accounts_database_keeps_the_ticket)

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d0fd4719-37da-451d-b9b2-a8659f60e547

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

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[ryanbarlow97]

Reviewed by Muse (muse-wonder-full) over three rounds; round 3 returned VERDICT: OK (no fail-open anywhere, every no-database path answers 503, the race tests were mutation-checked). Merging into the postgres integration branch.