feat(db): linking, skins, drinks and codes on native Postgres — #251

[ryanbarlow97]

Moves Linking and Skins, drinks and codes to native Postgres for #251, on the integration branch. Codes, links, Microsoft attempts, Profile/upload sessions, submissions, catalogues, warnings, notices and war declaration codes use src.database, %s, dict rows, RETURNING, ON CONFLICT, Jsonb and native boolean/timestamp handling.

The original Claude conversion and its authorship are preserved through the rebase: the former 03cac2804ead1b2fdf14cc21f46f9d814a0aa418 was 5c1a735a1f55fed67a45f7d20e023a71d5794915 on the ledger base and is now 73973e4f on the Patreon/LuckPerms base. Codex (Sol) resumed the existing worktree, completed the inherited fixtures and API checks, fixed the expiry check and two reproduced deadlocks, and performed final validation. New Codex edits are attributed to Codex in the follow-up commits.

Files and boundaries

• Converted: skins/{codes,discord_link,submissions,drinks,catalog,moderation,notifications,plugin_notices,review_sheet,drink_review_sheet,seed_dev_code}.py, auth/microsoft_link.py, and wars/declare_codes.py.
• Removed accounts bridges in auth/{users,view_as,admin,players,discord_names}.py. drop_view_leftovers, view_still_open, _cancel_microsoft_attempts, _link_for_player, the former _players_for and _with_minecraft reads, and refresh_all now use native Postgres. Account/Discord-link joins have explicit column aliases.
• Shared files have the minimum boundary changes: characters/lore_items.py reads submissions from Postgres; Patreon and LuckPerms, now merged in feat(db): Patreon, LuckPerms and diagnostics on native Postgres — #251 #258, read links on their native Postgres caller connections. Their remaining link fixtures use Postgres. dbstudio/studio.py and scripts/ps-preview.sh retain the later Studio/host-plumbing boundaries.
• The two async upload routes run database work in the thread pool. auth/admin_cli.py drops its obsolete SQLite --db option. The development seed command migrates Postgres and supports repeat runs.
• Test imports use the canonical src.* modules instead of replacing/reloading module identities; file paths are set explicitly. The SQLite legacy armour upgrade test remains against SQLite, which still prepares old data for the later copy tool. skins/db.py remains for the unconverted areas and final SQLite-removal step.

Independent Sol review round 3 — current implementation and integration

Fixes both R3 findings from a5cdc0fa: caller-ordered apply deadlocks (P2) and JSONB recipe/armour-object byte order (P3). All verified R1/R2 expiry and lock-order protections remain. R3 was committed before rebasing, then the complete branch was rebased onto Patreon/LuckPerms #258 at 8cb27b4b8c208a09a4094c20a67e4b6f0a1aadcb, as requested.

Batch locking: mark_applied and mark_drinks_applied fetch and lock every existing target row with ORDER BY id FOR UPDATE before their first mutation. Processing and responses still follow the original input order. Blank IDs, duplicate IDs, missing rows, allowed-status checks and applied_at IS NULL guards are preserved. Drink notifications and Characters promotion still run after commit.

The owned batch audit also reproduced reversed-input deadlocks in both remember_discord_usernames and remember_discord_nicknames. These validate inputs first, prelock all valid Discord IDs by discord_user_id, then process the retained original sequence, preserving duplicate counts, last-write behaviour, nickname clearing and the username overwrite guard. Plugin-notice and moderation acknowledgements also prelock their whole target set by numeric id before bulk mutation, so their order is independent of the update plan; duplicate response order and original delivery times remain unchanged.

The other audited paths do not add a reversed-input edge: grace candidates use separate transactions, sibling-code invalidation is serialised by the existing identity lock, singleton catalogue upserts and individual notification acknowledgements target one row, and view cleanup already prelocks sessions by id under the placeholders lock. The Characters post-apply write remains its existing post-commit SQLite boundary. No other area branch was changed.

Object bytes: the shared drink _public_row restores the fixed field order written by _validate_recipe; partial stored objects remain partial and extra fields/values are retained. Both /drinks/plugin/pending-apply and /drinks/plugin/drinks/{id} use it. _row_json_object restores tier aliases and staff tier scrolls from the retained tiers JSON array; tier base-set output also retains that array order. Non-empty wire assertions exercise real create→approve→GET flows, staff and player armour, the approved queue and single-item plugin endpoints, including non-empty recipe ingredients/effects/lore. Return types and values are unchanged; no order columns or schema expansion were introduced. The R2 plugin-notice schema ordering is retained.

Tests and mutations: test_batch_and_wire.py adds 29 cases. Twenty exercise concurrent real HTTP writers and observe pg_stat_activity.wait_event_type = 'Lock', with timing gates after either the actual prelock or the first UPDATE. Both same-order controls and reversed [A,B]/[B,A,C] batches finish 200/200; C is applied and drink notices are not duplicated. Tests also pin caller-ordered responses, blank/duplicate/invalid handling, conditional states, delivery idempotency, notification visibility after commit, and non-empty raw plugin bytes. No test holds a review-created row or table lock.

The initial 19 cases against a5cdc0fa produced eight assertion failures and eleven controls passed: four reversed-input HTTP 500s (skin/drink apply, username/nickname) and four byte mismatches. Ten prelock-phase cases were then added to distinguish sorted prelocks from caller-order prelocks. On the final integrated tree, 15/15 focused mutation configurations are caught: remove the four apply/name prelocks, replace the six ordered apply/name/ack prelocks with caller-order locks, sort the two caller-ordered apply outputs, or bypass recipe/alias/scroll ordering. Every selected mutant fails by assertion, with no collection/setup errors. Mutations only run in an ignored source copy and are restored there.

Integration resolution: the conflicts kept #258's native Patreon/LuckPerms implementations. A separate Codex commit removes their last temporary SQLite Discord-link reads, passes the native caller connection through the helpers, and converts the corresponding link fixtures. All seven rebased commit authors, messages and co-author trailers were checked against their original commits and preserved. The original Claude work remains attributed to Claude; the new R3/integration work is attributed to OpenAI Codex.

auth/users.py::session_user_in is unchanged from 8cb27b4b (AST comparison): it locks the user before the session, verifies the session still belongs to that locked user, and rechecks expiry after waiting. Our native view/Profile and Microsoft checks remain intact. Shared locking/Microsoft/view and native integration gates pass on this combined tree.

Independent Sol review round 2 — historical implementation follow-up

Fixes all four findings from the independent review at e0ee3a23. Prior commits and attribution are preserved, including the verified round-1 fixes. At that round the base was ledger #254 at 9a3835bbdd55e55c941470a57ff2b0c7b83021d0; the current integration base is #258 below.

1. Expiry throughout session replacement: skin/drink redemption and site reuse recheck fresh UTC after deleting old sessions and resetting the held code, before inserting the new session. They also check after the session insert. Expiry rolls back the deletion, redemption reset and insert together. Site start restores the old session in a savepoint before computing its existing 409 {detail: {reason: "cooldown", next_at: ...}} response; it does not let an expiry exception escape as HTTP 500. Session TTL starts after the old-session wait.
2. Discord completion: the identity advisory locks still come first, followed by an explicit code-row FOR UPDATE before reading UTC. The conditional spend retains its single-use predicate. A fresh expiry check after _bind covers later link-insert and sibling-code waits and rolls back the permanent link, all code spends and the outbox notice together.
3. Plugin bytes: the four known plugin-notice payload schemas are reconstructed in their legacy insertion order at serialisation. guild_left_grace is discord_user_id, grace_until, left_guild_at; grace_expired keeps discord_user_id before grace_until. These producers have flat payloads, so there is no nested schema to invent. Extra fields are retained. JSONB storage and schema are unchanged.
4. War redemption: lock the code before reading UTC, retain the guarded single-use update, and check fresh expiry again after that update. Success and single-use rejection bytes are pinned through the actual API route.

The broader expiry audit also covers submission inserts and drink-texture reference updates, Profile-session insertion, Microsoft binding after its initial state/session check, Microsoft consumption after its UPDATE, revocation after its UPDATE, and staff-view Profile creation after its shared-row wait and later writes. Those checks now run after the last relevant wait and roll back partial mutations on refusal. Guild-leave grace checks read UTC after the link lock, and leave/join status uses a fresh clock after writing its notice.

Lock order is unchanged: site start takes cosmetic_mint:<uuid> → code → sessions; submissions hold their code through their other writes; binding takes discord_link:<id> → minecraft_link:<uuid> → code/state/session/link rows. Repeated Microsoft/view validity checks reuse the rows already held FOR SHARE; they add no reverse lock edge. Native expiry/revocation queries do not lock consumption rows. The existing sibling-link, unlink/callback, and site-reuse/redemption deadlock regressions remain in the focused gate.

Changed implementation files are skins/{codes,discord_link,submissions,drinks,plugin_notices}.py, auth/{microsoft_link,view_as}.py and wars/declare_codes.py. The shared view_as change is confined to this area's existing view/Profile bridge. There are no schema changes and no added or removed PG-MIGRATION marks in this follow-up.

test_expiry_waits.py adds 22 real wall-clock lock-wait cases. Each observes pg_stat_activity.wait_event_type = 'Lock', confirms the attempt began before the stored deadline, holds the blocker until actual UTC passes that deadline, and checks the outcome and rollback. They cover old-session row waits (skin/drink redemption and site reuse), site-start API refusal bytes, Profile/session inserts, Discord completion's code/link/sibling waits, Microsoft state/session deadlines during binding, view expiry, submission inserts, drink-texture row waits, war row/UPDATE waits, Microsoft consumption, revocation and guild grace. Row and advisory races already covered by round 1 remain enabled; table locks isolate later DML waits without changing production code.

test_api_formats.py now has 13 raw JSON-byte fixtures, including the multi-key known notices and actual Discord/war completion plus single-use responses. Two additional raw-byte assertions exercise raced site-start API refusals. Normal parsed fields and timestamp formatting are unchanged.

Focused mutation validation runs only on an ignored source copy. 18/18 mutation configurations are caught: late expiry checks for each redemption/reuse path, submission spending, Discord's first and later waits and single-use predicate, Microsoft final authorisation/consumption, view validation, war first/later waits and single use, revocation, guild grace, and notice key order. Where an intermediate and final guard back each other up, the mutation removes both; this is reported as one configuration. Every selected case must fail by assertion, with no collection/setup errors. The real production source is never mutated by this harness.

Independent Sol review round 1 — historical implementation follow-up

Rebased onto ledger #254 at 9a3835bbdd55e55c941470a57ff2b0c7b83021d0. The round-1 commit changed only auth/microsoft_link.py, skins/codes.py and skins/test_locking.py.

1. Microsoft expiry after waits: consume locks the state before reading UTC, checks expiry, then performs the single-use guarded update. still_valid holds the state and originating cookie session FOR SHARE, reads both expiry values, and compares them against fresh UTC after acquiring both locks. An attempt that expires during an upstream call or a row wait cannot authorise a permanent link.
2. Profile redemption after expiry: the Profile code is locked FOR UPDATE before reading UTC. The guarded consumption and session insert remain in that same transaction, so an expired code cannot open a remembered 30-day session. The adjacent skin/drink redemption and revocation paths now read UTC after their existing code lock too.
3. Safe site-code reuse: _open_code remains a read-only candidate lookup for allowance reads. The write path explicitly locks the selected code by id before touching sessions, then rechecks revocation, expiry and consumption with a new statement/snapshot. If that candidate stopped being usable, it goes through the existing fresh cooldown/mint decision instead of resetting its redemption or issuing a bearer for it.

The audited lock order is cosmetic_mint:<uuid> (site start only) → codes row → sessions. TFMCWeb skin/drink redemption and Profile redemption take the code first; revocation and submission consumption also begin with the code. The shared session-reset helper now documents that precondition. Plain session revocation and view cleanup delete sessions without subsequently locking codes, so they do not add a reverse edge. Submission rollback writes its code after deleting the submission, while the redemption/reuse checks read submission consumption without locking submission rows. Microsoft state start/consume/finish/cancellation do not subsequently lock cookie sessions; callback validation retains its existing shared locks and the existing identity advisory-lock ordering.

Fifteen new parameterised regression cases cover Microsoft state/session expiry during an observed row wait, state consumption expiry, Profile/skin/drink redemption expiry, revocation expiry, site reuse expiry, revocation/consumption landing after candidate selection, and both skin/drink reuse-versus-redemption deadlocks. The round-1 expiry tests advance a controlled UTC clock only after pg_stat_activity confirms a real Lock wait; they do not rely on sleeps or scheduler timing. The two deadlock cases use full production calls with an existing upload session. Fourteen cases were run before the fix and all failed, including two actual DeadlockDetected errors; the adjacent revocation case was added afterwards.

At the round-1 head, the original eight exact JSON-byte fixtures passed; round 2 strengthens the payload fixtures below. The existing invalid-code/cooldown/callback outcomes are retained for newly refused raced requests.

Locking recipes and regression proof

The original race tests in the table below are in backend/src/skins/test_locking.py. Each uses two real Postgres connections and observes pg_stat_activity.wait_event_type = 'Lock' before releasing the holder. They assert the relevant row or advisory wait, then the resulting data and notices. Workers are released and joined even when a mutation makes a wait assertion fail.

Recipe	Tests
A: skin/drink insert locks the code and revalidates revocation, expiry and consumption in the insert transaction	test_a_skin_code_is_consumed_by_one_submission, test_a_drink_code_is_consumed_by_one_submission, test_a_submission_waits_for_a_revoke_of_its_code, test_a_submission_rechecks_expiry_after_its_code_lock_wait (skin and drink)
D: cosmetic_mint:<lower(uuid)>, across realms, in both mint paths and staff resets	test_a_site_start_waits_for_a_tfmcweb_mint, test_two_site_starts_mint_one_code, test_every_mint_path_takes_the_cosmetic_mint_lock
C: one internal lore-upload code per player	test_two_first_kit_uploads_share_one_code
A + B: lock a Profile code before fresh expiry validation and single-use consumption; B for Discord codes	test_a_profile_code_is_redeemed_once, test_a_link_code_is_spent_once, test_link_redemption_rechecks_a_code_spent_while_waiting
C: _bind never replaces either side of an existing link	test_a_link_waits_for_a_link_being_made_of_the_same_discord_account, test_a_link_waits_for_a_link_being_made_of_the_same_player
C: one Microsoft attempt per user; A + B: lock state before fresh expiry validation, spend it once	test_two_microsoft_starts_keep_one_attempt, test_a_microsoft_state_is_spent_once
A: hold Microsoft state and originating session FOR SHARE through the link write	test_signing_out_mid_attempt_stops_the_microsoft_link, test_unlinking_mid_attempt_stops_the_microsoft_link, test_a_sign_out_waits_for_a_microsoft_link_being_written
A: hold the staff view through Profile-session creation	test_ending_a_view_waits_for_its_profile_session_and_removes_it, test_a_profile_session_is_refused_once_its_view_ended
B: expire a grace and enqueue its notice once; A: one guild-leave grace	test_two_grace_expiries_notice_once, test_a_grace_expiry_does_not_remove_a_rejoined_player, test_two_guild_leaves_start_one_grace
B: denial cannot undo an approval; atomic texture reference decrement	test_a_denial_never_undoes_an_approval, test_two_releases_of_a_shared_drink_texture_both_count
B: one-use war declaration code and conditional revocation	test_a_war_code_is_redeemed_once, test_a_war_code_redeemed_meanwhile_is_not_revoked
D: order identity writes before code/state/link row locks	test_two_sibling_link_codes_do_not_deadlock, test_unlink_waits_for_a_microsoft_refresh_of_the_same_pair (unlink by Discord and UUID)

The last row addresses two races reproduced as DeadlockDetected before the fix: two sibling code redeemers each holding a code while invalidating the other's, and unlink holding the link row while Microsoft holds its state. Bind transactions now take discord_link:<discord id> then minecraft_link:<lower(uuid)> before any row lock. Unlink and each conditional grace expiry take the Discord key before deleting the link and cancelling attempts. Grace expiry rechecks eligibility after waiting, so a rejoin is retained.

Historical round-1 mutation validation on a separate ignored source copy removed or weakened 35 protections: the original 21 recipes plus 14 guards for this review follow-up. Every selected case fails by assertion; no mutation survives and none is counted as caught merely because collection or fixture setup errored. Production files are not mutated. The original identity deadlock regressions and the new site-reuse/redemption regressions were reproduced with real deadlocks before their respective fixes.

PG-MIGRATION marks

All Linking and Skins marks are removed, including the accounts bridges and, after integrating #258, the four remaining Linking boundaries in luckperms/{changes,views}.py and patreon/{service,linking}.py. No marks are added by the R3 fixes. Eleven marks remain: Characters (the roster/lore bridges), Studio and host plumbing. Patreon/LuckPerms and their link reads now share Postgres; the remaining Characters bridge still spans databases.

API compatibility and behaviour

• Parsed client timestamps retain YYYY-MM-DDTHH:MM:SSZ, including truncation of stored microseconds. Native datetimes are formatted explicitly; JSONB remains structured JSON; flags remain JSON booleans.
• The original eight tests in test_api_formats.py pin exact JSON bytes for identity status, TFMCWeb's mint status and active codes, staff code inspection, plugin notices, approved skins, drink application queues and war codes. test_times_leave_in_the_api_form also checks newly issued/session/grace timestamps.
• The approved-skins since parameter is validated before SQL; malformed input returns 400 instead of a Postgres cast error.
• Following feat(db): native Postgres foundation (database package, schema, tests) — #251 #252, an upload whose session remains live but whose code expires is now refused with Code has expired. Both upload paths check this after taking the code lock and after their remaining database writes; this is an explicit tightening of the previous session-outlives-code behaviour.

Validation provenance

• Current exact head: 98b655f85898d12f09feb9dd580e26b5bd5ba589. Current base: 8cb27b4b8c208a09a4094c20a67e4b6f0a1aadcb (feat(db): Patreon, LuckPerms and diagnostics on native Postgres — #251 #258), which includes ledger feat(db): chronicle and ledger on native Postgres — #251 #254. The tracked code/tests validated locally are byte-identical to the committed tree, including the native integration adapters.
• Current focused R3/R1/R2 gate: 111 passed in 55.08 s (.scratch/sol-round3/focused.{log,xml}).
• Current shared locking/Microsoft/view gate: 239 passed in 64.79 s (.scratch/sol-round3/shared-auth.{log,xml}); includes auth locking, Microsoft, views, skin linking/expiry, LuckPerms locking, Patreon locking/expiry.
• Current native adapter/fixture integration gate: 101 passed in 12.49 s (.scratch/sol-round3/integration.{log,xml}), including Patreon's existing byte fixtures.
• Current expanded affected gate: 1012 passed, 11 subtests passed in 110.24s (0:01:50) (.scratch/sol-round3/affected.{log,xml}). Covers all skins, auth, Patreon and LuckPerms tests, war codes and the related API/Characters boundary tests.
• Current R3 mutation proof: 15/15 caught; .scratch/sol-round3/mutations.log and mutations/mutation-*.{log,xml}. Each selected test must fail by assertion. Production sources are not mutated.
• Historical R3 pre-fix reproduction: first 19 cases against a5cdc0fa: 8 assertion failures, 11 controls passed in 7.28 s. The pre-integration gates were 111 focused passed, 902 affected passed, 11 subtests passed, 15/15 mutations caught; preserved in .scratch/sol-round3/pre-rebase/. They are not claimed as new-base validation.
• Historical round 2: a5cdc0fa8aabbeefb8345db29e4e3d0fec945b56 passed the full Build 38983885649. Its local gates were 82 focused, 873 affected plus 11 subtests, 18/18 mutations caught. R1/R2 were independently verified again by the parent review; this does not replace current-head CI.
• Historical original baseline: 7d2a1ee7, 2,454 passed, 4 skipped, 175 subtests passed (/tmp/ps-skins-baseline.log). The earlier full local area suite at 0b365a35 was 2,494 passed, 4 skipped, 175 subtests passed. Both precede the integration changes and review fixes.
• git diff --check passed. Native gates used only the restarted, health-checked ps-skins-pg on port 55435. Task-owned temporary files used /dev/shm and are cleaned up; logs remain in the ignored artefact directories.
• Full current-head CI: Build 38985959274 was triggered for the exact head above. The parent owns CI monitoring and the next independent review. No duplicate full local backend suite was run or claimed.

Only automatic Preview 38985956088 for the final push was cancelled, before deployment; Build was left running. ps-skins-pg is stopped (running=false, status=exited), and the worktree is clean. No SSH, deployment, merge or GitHub comments were performed. Remaining migration boundaries are Characters, Studio and host plumbing/cutover, outside this follow-up.

[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: 4b959d51-a65b-442b-91d3-eddb98e99e74

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