feat(db): Patreon, LuckPerms and diagnostics on native Postgres — #251

[ryanbarlow97]

Converts Patreon, LuckPerms and diagnostic reports to native Postgres for #251, following the recipes in #252 and the accounts conventions in #253. Changes to LuckPerms and their audit entries now commit together. A conditional UPDATE … RETURNING spends each Patreon confirmation token once, fixing the ignored-row-count double-confirm bug.

This resumes the interrupted conversion in the existing worktree. The recovered Claude implementation is preserved in 28be41c6 with Claude attribution. 69d50019 contains only the resumed Codex/Sol changes, with Codex attribution. Existing foundation/accounts authorship is unchanged.

Files and types

• luckperms/{mirror,changes,commands,views}.py: native queries, COPY for complete snapshots, JSONB and boolean values, timestamp conversion, consistent reads, mirror/change locking and atomic staff audit entries.
• patreon/{service,client,linking}.py: native queries, leases, creator-token CAS, advisory locking, OAuth and confirmation consumption, JSONB outboxes, timestamp and boolean handling.
• diagnostic/reports.py and api/diagnostic_routes.py: native report storage and collision retries. Reject JSON values Postgres cannot store; preserve literal Unicode escapes. api/patreon_routes.py and the diagnostic POST move blocking database calls to the thread pool.
• Minimal shared accounts changes: remove the SQLite bridges from auth/users.session_user_in and auth/audit.record; move auth/account_overview.rank to the Postgres mirror. Their callers now use the same transaction. The obsolete SQLite-bridge test is removed; the rank test takes postgres.
• Area tests use the Postgres fixture; SQLite fixtures remain only for the unconverted Linking/Profile data they still need. backend/benchmarks/luckperms_snapshot.py reproduces the snapshot timing.

Recipes and proof

Races use separate real Postgres connections and assert pg_stat_activity.wait_event_type = 'Lock', with the expected advisory or row wait, before releasing the holder. Tests live in luckperms/test_locking.py, patreon/test_locking.py and diagnostic/test_routes.py.

• X5 snapshot revision lock: test_two_snapshots_apply_one_after_the_other.
• Queue seeds lp_state, takes it FOR SHARE, then locks the actor session/user (X1), then change rows. One consistent mirror: test_a_change_is_checked_against_one_mirror. One open target change (partial unique index C): test_two_changes_to_one_target_queue_once. Atomic audit and demotion exclusion: test_a_demotion_waits_for_a_change_and_its_audit_row.
• Poll upsert locks lp_state first; pending actors are locked in id order before any change settlement. _current_actor itself reads FOR SHARE. Tests: test_a_poll_locks_actors_before_timing_out_changes, test_current_actor_itself_waits_for_a_demotion, test_a_demotion_landing_first_cancels_the_change, test_a_demotion_waits_for_a_change_being_handed_out.
• Poll conditional update B: test_a_change_is_handed_out_once, test_two_polls_hand_a_change_out_once.
• Results lock lp_state before change rows, then settle with a conditional update/RETURNING. Both directions are exercised by test_a_result_and_a_poll_never_deadlock and test_a_poll_and_a_result_never_deadlock; test_a_result_settles_a_change_once isolates B. Timeout updates retain the expected status: test_a_timeout_never_overwrites_a_result_landing.
• X4 patreon advisory lock around every compute/plan path and every listed X4 site: test_every_recompute_takes_the_patreon_lock_first parametrises create/update, recompute, unlink, brake release, staff link, import, dispatch, ack, full sync, member refresh and confirmation. Dispatch versus cancellation and subject uniqueness also race in test_a_dispatch_never_sends_a_change_being_cancelled and test_two_patrons_cannot_claim_one_discord_account.
• Lease C (ON CONFLICT … DO UPDATE … WHERE expires_at <= … RETURNING owner): test_an_expired_lease_is_taken_once.
• Sync lease FOR UPDATE: test_a_sync_stores_nothing_once_its_lease_is_taken_over. Expiry is rechecked after waiting for X4: test_a_sync_rechecks_lease_expiry_after_waiting_for_the_patreon_lock.
• Token-refresh lease FOR UPDATE and token CAS: test_a_token_refresh_waits_for_a_lease_takeover, test_a_token_refresh_never_overwrites_tokens_rotated_meanwhile; both observe a concurrent row wait.
• _sync_failure begins with its atomic counter UPDATE/row lock: test_two_failures_count_twice_and_alert_once. alert() uses ON CONFLICT (kind) WHERE acked_at IS NULL DO NOTHING: test_an_alert_raised_twice_at_once_is_kept_once.
• OAuth-state B: test_an_oauth_state_is_spent_once. Confirmation B: test_two_confirmations_link_once, test_a_cancel_and_a_confirm_cannot_both_spend_a_token, and test_simultaneous_confirmation_consumes_token_once in test_linking.py.
• No Patreon HTTP inside a lock-holding transaction: test_patreon_is_called_with_no_transaction_open, plus the token-refresh mock HTTP handlers' transaction checks. HTTP finishes before the write transaction opens.
• Diagnostic code C: test_two_reports_retry_a_colliding_code observes the conflicting insert wait and successful retry.

22 deliberate mutations were checked in an isolated source copy, restoring each after its test. Every mutation was detected: X5, queue SHARE, open-change index, X1, actor pre-lock order, _current_actor SHARE, poll/result/timeout B, all 11 X4 sites, lease C, both lease row locks, token CAS, failure counter, alert upsert, OAuth B, old ignored-row-count confirmation logic, HTTP inside the transaction, expiry recheck, and diagnostic collision upsert. Removing the result's initial lp_state lock reproduced a real Postgres deadlock. The reverted confirmation logic produced two successful confirmations. Mutation artifacts remain local under .scratch/sol/mutations/.

Output compatibility

luckperms/test_wire_format.py and patreon/test_wire_format.py pin responses captured from the SQLite implementation at 7d2a1ee7. TFMCWeb snapshot/unchanged/poll/results replies and Patreon plugin, Discord-bot and status responses remain byte-identical, including operation key order, outbox order, flags and timestamps. LuckPerms retains …Z; Patreon retains datetime.isoformat() (…+00:00, including microseconds). The commands-view wall-clock timestamp is normalised for comparison.

Staff Patreon lookup and diagnostic report JSON objects retain their values but may have JSONB key order; those are compared structurally. Lookup's active remains 0/1. Repeated grace periods and link-event identities retain their stored string form, avoiding spurious generation changes and DMs.

Migration marks and integration limits

Removed all six LuckPerms-owned marks: the accounts session_user_in and audit.record SQLite bridges, account-overview rank, LuckPerms _account, _target_account_role and _current_actor bridges. No remaining PG-MIGRATION mark names LuckPerms, Patreon or Diagnostics.

Added four Linking-owned marks, for reads of discord_links/discord_link_codes in changes.discord_link, views._account, service._discord_links and linking._known_name. These remain SQLite reads until the parallel Linking PR converts them, so those cross-store link reads are not yet atomic with Postgres transactions. No other area is converted here; no schema migration, deployment or merge is included.

Original conversion timings and test results (before round 1)

Local pgvector/pgvector:pg17, port 55437, Python 3.12. A 2,000-user snapshot contains 20,000 player nodes, 40 groups (1,199 group nodes) and 6 tracks; seven replacements over a populated mirror include parsing, locking, deletes and writes:

• COPY: median 233.9 ms, min 193.8 ms, max 347.1 ms.
• Executemany comparison: median 1,245.3 ms, min 1,028.6 ms, max 1,622.9 ms.

Measurements were on the shared local test host while verification ran; they are comparative timings, not production latency guarantees.

• Saved pre-change postgres baseline (7d2a1ee7): 2,454 passed, 4 skipped, 175 subtests passed in 1,518.88 s. Reused the completed /tmp/ps-patreon-baseline.log; did not repeat it.
• First recovered locking pass: 54 passed in 81.90 s.
• Pre-review affected suite (src/luckperms src/patreon src/diagnostic src/auth): 676 passed in 775.57 s.
• Pre-review full backend suite with TEST_DATABASE_URL on port 55437 at a4f48ed9: 2,503 passed, 4 skipped, 175 subtests passed in 1,301.17 s (21m41s), an increase of 49 passing tests over the saved baseline.
• GitHub Build passes on the same HEAD: backend tests, frontend lint/format/tests and production build. Frontend report: 2,325 passed, 1 skipped. The automatic Preview deploy check is cancelled, as explained below.
• Production modules pass Pyflakes; git diff --check passes. Test-only fixture imports follow the existing pytest pattern.

No external Patreon requests were made; HTTP is mocked. No SSH, manual deployment, merge or changes to other area branches. The requested branch push triggered the repository’s automatic Preview workflow (run 38979079517). It was cancelled to honour the no-deploy instruction, but its logs show pv-pg-area-patreon-lp-{backend,frontend}-1 had started before cancellation took effect. The parent must check/clean up that automatic preview; no SSH was used here. The named local test container ps-patreon-pg was stopped after verification; no other container was stopped.

Independent Sol round 1 — implementation follow-up

Rebased the existing conversion onto 9a3835bbdd55e55c941470a57ff2b0c7b83021d0 (merged ledger #254), preserving author names, attribution and author dates. The two preserved conversion commits are now 28be41c6 and 69d50019; this follow-up is Codex/Sol work.

Follow-up HEAD: 23715a30effa06c640208c0f3cf041e5531beb5f. All three reproduced P2 findings are fixed:

1. Actor user/session lock inversion. auth/users.py:session_user_in(lock=True) first locks only the user row through a non-locking session lookup, then locks the session. The second query binds the session to that already-locked user, re-reads role/session data and checks expiry after any wait. Ordinary reads still take no row lock. LuckPerms keeps lp_state first and lp_changes last; its final expiry re-read takes no new row lock. Successful changes and their audit entries still commit together. Expiry after a later change-row wait rolls back the queued change and records a refusal.
2. OAuth and confirmation expiry after waits. consume_state and consuming _pending lock the token row before reading a fresh clock, then use the existing conditional UPDATE … RETURNING in that transaction. The double-confirm/cancel guard remains. Confirmation also checks fresh expiry after waiting for X4 before linking; a token already spent before that wait remains spent. New OAuth states and confirmations compute their full TTL after cleanup waits.
3. Lease expiry and deadline clocks. Acquisition seeds/locks the named lease with a no-op conflict update, then evaluates fresh time and runs the conditional takeover upsert. This also handles a concurrent insertion or release without exposing a placeholder lease. Renewal locks the lease row before evaluating expiry and computing its deadline, retaining owner and expiry CAS. Expired owners cannot revive leases; successful acquisition/renewal gets its full TTL after the wait.

The analogue audit also moved token-refresh expiry/deadline checks after both lease and token row locks, sync time after the X4/lease waits, default link timestamps after X4, and LuckPerms poll timeout evaluation after state/actor locks. No external HTTP was moved under locks. Mock handlers assert that application transactions are closed when they run.

Deterministic proof: patreon/test_expiry_locking.py and the new cases in luckperms/test_locking.py advance a controlled clock only after pg_stat_activity confirms the competing backend's real row/advisory lock wait. They test real admin.change_role and admin.revoke_sessions in both directions, user/session expiry, later change-row expiry, OAuth/confirm/cancel expiry, X4 confirmation expiry, acquisition behind insertion/release, expired/live renewal and full TTLs, owner revalidation, token-row expiry, and sync/mint/link/poll analogues. Existing wire-format and atomic-audit checks remain in the affected gate.

• Focused locking gate: 84 passed in 74.29 s.
• Round-1 affected gate: 485 passed in 421.33 s (7m01s), including Patreon, LuckPerms, Diagnostics and the relevant accounts locking/admin/view-as/staff/auth-route tests. Both wire-format modules pass.
• 18/18 isolated mutations detected. Reverting the joined session/user lock reproduced DeadlockDetected against both actual staff paths. Removing fresh clocks/row locks or the original owner/token/used-at guards made the corresponding regression tests fail. Mutation sources were restored after each probe; they never modified the implementation checkout. Evidence: .scratch/sol-round1/.
• Full Build is delegated to exact-head CI after the final push; the parent watches it. No duplicate local full-backend run or CI polling is performed for this follow-up.

Shared integration hunk: only auth/users.py:session_user_in is newly changed outside the owned areas. When integrating the Skins branch's separate auth edits, retain its unrelated changes and preserve this user-before-session lock sequence, the expected-user predicate and the fresh expiry check. .scratch/sol-round1/auth-integration.patch contains that hunk independently. The existing four Linking-owned bridges still await the parallel conversion; no bridge is added back here.

No new schema changes, API fields or time formats. No agents, SSH, manual deployment or merge. The automatic Preview run from this push (38982462894) was cancelled before deployment; full Build is left to the parent without polling it. Only ps-patreon-pg was used locally, and it is now stopped.

Independent Sol round 2 — implementation follow-up

Current follow-up HEAD: d24731b0e8e6ce332ea47845c4480c251022d2b4. The existing conversion and round-1 authorship are preserved on base 9a3835bbdd55e55c941470a57ff2b0c7b83021d0. This follow-up is Codex/Sol work. Earlier sections' test counts and HEADs are historical; the evidence below describes this follow-up.

All four round-2 findings are addressed:

1. LuckPerms pending TTL after settlement waits. Timeout settlement refreshes its cutoff between sent/pending phases. Poll locks pending rows before checking each candidate against fresh time, then conditionally expires or dispatches it. This covers waiting on an overdue sent row, overdue pending row or the candidate itself. lp_state remains first, users/sessions precede lp_changes, and dispatch keeps its status CAS. The analogue check also moved queueing's final session-expiry validation after the atomic audit insert; an audit-table wait cannot commit an expired-session change.
2. Expiry after later write-table waits. OAuth consumption, pending-token consumption/cancellation, lease renewal and token rotation now acquire the same ROW EXCLUSIVE table lock their write requires before row locking and sampling UTC. This mode is compatible with ordinary concurrent DML; it adds no global permissions lock. It closes the reviewer's controlled SHARE-table interference case, which permits a row lock while blocking the later write. Owner, token and single-use CAS guards remain. Valid renewals and rotated tokens receive a full TTL measured after the wait. Acquisition already takes its actual write/row locks through the seed upsert before its time sample.
3. Confirmation expiry during link/outbox writes. Confirmation spends its token inside the X4/link transaction and validates fresh expiry after the mutations. Expiry rolls back the token, links, desired state and outboxes together. A savepoint preserves the existing ordinary-error contract: valid-token errors consume the token but roll back link mutations. The expiry-only rollback intentionally supersedes round 1's spent-token expectation. For unknown patrons, consent/linking commits while valid before the existing HTTP fetch; later member refresh uses its own sync lease. No HTTP is moved under locks.
4. Sync lease expiry during later mutations. Sync validates the held lease's owner/deadline again after member, entitlement, outbox and success-state writes, before commit. Failure rolls those writes back together. The separately committed _sync_failure handler still records failure metadata, including when another real failure writer caused the wait; its committed counter is preserved. The lease row remains held, preventing ownership changes until the transaction ends.

Regression proof and provenance: patreon/test_later_waits.py promotes and extends the independent reviewer's nine probes from /tmp/sol-review-patreon-r2-probes.py. Every clock advance follows an observed real Postgres wait in pg_stat_activity. Tests include link/outbox and member/outbox row waits, controlled table-lock waits, the actual _sync_failure writer overlap, full entitlement snapshots on rollback, expiry/error token-spend distinctions, timeout read/poll waits and full fresh TTLs. The table-lock probes are controlled interference, not claims that ordinary application DML takes SHARE table locks. Existing actual staff demotion/revocation, wire-format, privacy and atomic-audit tests remain in the affected gate. R1's shared auth/users.py hunk is unchanged; no further shared auth edits are introduced.

• Round-2 affected gate (Patreon, LuckPerms, Diagnostics and relevant accounts tests): 503 passed in 391.26 s (6m31s). Two final regression cases added after collection passed separately: 2 passed in 4.23 s.
• Focused locking/expiry/compatibility gate: 129 passed in 161.13 s; the same two final cases passed separately as above.
• The nine original external reviewer probes, unchanged: 9 passed in 10.66 s. Explicit plugin loading emitted one pytest assert-rewrite warning; no application failures.
• 14/14 isolated mutations detected: the four required table locks, final confirmation expiry/atomic spend, ordinary-error savepoint, final sync deadline (four failing cases including the actual failure writer), final poll TTL, per-phase timeout clock, post-audit session expiry, and OAuth/confirmation/token CAS guards. Every mutated run failed its selected regression; mutations ran in a separate source copy and never changed the implementation checkout.
• Production Pyflakes and git diff --check pass. Local evidence: .scratch/sol-round2/, including affected JUnit XML and each mutation's failure log. No duplicate local full-backend suite: exact-head Build is triggered by the push and left to the parent to monitor; its result is not claimed here.

No schema, API-byte or timestamp-format changes; the existing four Linking-owned bridges remain. No new agents, SSH, merge or manual deployment. Only ps-patreon-pg was used for local Postgres and it is stopped. The push's automatic Preview run 38984190405 was cancelled before deployment. No unresolved implementation doubts; the four findings are ready for a new independent review, with full Build still owned by the parent.

[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: b0e942a6-e1ae-4bfc-a578-b400f83d57ce

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.}