fix(server): transactional sync push; single-use refresh rotation #136

2026-07-07T01:00:01Z

funman300 commented

2026-07-07 01:00:01 +00:00

Review findings M1 and M2 (2026-07-06 large-scale review).

M1 — sync push race. push ran load→merge→store as three separate DB operations; two devices pushing concurrently both read the same stored payload and the second store overwrote the first merge (self-healing later thanks to the monotone merge, but the server regressed in between). The whole cycle — including the leaderboard conditional update — now runs in a single transaction. Also fixes the leaderboard helper's stale docstring (claimed one conditional UPDATE; was two statements).

M2 — refresh rotation not single-use under concurrency. The SELECT-then-DELETE sequence let two concurrent refreshes both succeed with the same token. The rotation now gates on rows_affected == 1 from the DELETE itself (the SELECT precheck is removed — rows_affected == 0 covers never-existed, deleted-account, and concurrently-consumed alike). Sequential-reuse behaviour is unchanged and still covered by consumed_refresh_token_is_rejected.

Helpers were made executor-generic (impl Executor<'_, Database = Sqlite>) so pull keeps passing the pool while push passes its transaction.

Gates: cargo clippy --workspace --all-targets -- -D warnings clean; cargo test --workspace green.

🤖 Generated with Claude Code

Review findings **M1** and **M2** (2026-07-06 large-scale review). **M1 — sync push race.** `push` ran load→merge→store as three separate DB operations; two devices pushing concurrently both read the same stored payload and the second store overwrote the first merge (self-healing later thanks to the monotone merge, but the server regressed in between). The whole cycle — including the leaderboard conditional update — now runs in a single transaction. Also fixes the leaderboard helper's stale docstring (claimed one conditional UPDATE; was two statements). **M2 — refresh rotation not single-use under concurrency.** The SELECT-then-DELETE sequence let two concurrent refreshes both succeed with the same token. The rotation now gates on `rows_affected == 1` from the DELETE itself (the SELECT precheck is removed — `rows_affected == 0` covers never-existed, deleted-account, and concurrently-consumed alike). Sequential-reuse behaviour is unchanged and still covered by `consumed_refresh_token_is_rejected`. Helpers were made executor-generic (`impl Executor<'_, Database = Sqlite>`) so `pull` keeps passing the pool while `push` passes its transaction. Gates: `cargo clippy --workspace --all-targets -- -D warnings` clean; `cargo test --workspace` green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

funman300 added 1 commit 2026-07-07 01:00:01 +00:00

fix(server): transactional sync push; single-use refresh rotation 8eb316751d

Two concurrency fixes from the 2026-07-06 review (findings M1, M2):

sync push (M1): the load→merge→store cycle ran as three separate DB
operations. Two devices pushing concurrently both read the same stored
payload, merged independently, and the second store overwrote the first
merge — the server visibly regressed until the losing device pushed
again. The whole cycle (including the leaderboard update) now runs in
one transaction; SQLite serialises the writers. The leaderboard
helper's stale docstring (claiming a single conditional UPDATE that was
actually two statements) is corrected — the transaction now provides
the atomicity it described.

refresh rotation (M2): SELECT-then-DELETE let two concurrent refreshes
with the same token both pass the liveness check and both mint fresh
token pairs. The SELECT is gone; rotation now gates on the DELETE's
rows_affected — whoever removes the jti row wins, everyone else gets
401. Sequential reuse was already covered by
consumed_refresh_token_is_rejected, which still passes unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

funman300 merged commit 021c5d6ad8 into master

2026-07-07 03:04:41 +00:00

funman300 referenced this issue from a commit

2026-07-07 18:09:21 +00:00

fix(data): serialise token refreshes so overlapping 401s can't force a logout

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: funman300/Ferrous-Solitaire#136