Skip to content

fix(sync): progress compares client clock with itself; mobile retries failed reading sessions - #695

Merged
mrviduus merged 2 commits into
mainfrom
fix/progress-clock-and-sessions
Oct 4, 2026
Merged

mrviduus merged 2 commits into
mainfrom
fix/progress-clock-and-sessions

Conversation

@mrviduus

@mrviduus mrviduus commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Why

Architecture review 2026-10, #12: silent data loss.

What

  • Progress last-write-wins (UserDataEndpoints.UpsertProgress, both normal and insert-race paths) compared the client's UpdatedAt with the server-clock UpdatedAt → a device whose clock is behind had its newer writes rejected with 200. New ProgressClock: incoming client stamp (clamped to server now + 5 min) vs stored ClientUpdatedAt; null on either side → accept. UpdatedAt stays the server write time.
  • Migration ProgressClientUpdatedAt — adds nullable reading_progresses.client_updated_at only.
  • Shared updateProgress takes recordedAt; mobile now stamps record time (web already did).
  • Mobile reading sessions were dropped on any submit failure (.catch(()=>{})). Now a persisted AsyncStorage queue (reading.pendingSessions, cap 50, drop ≥6 days, serialised ops) flushed on submit, sign-in/app start and reconnect; cleared on sign-out. Pure rules in packages/shared/src/reader/pendingSessions.ts. Server already dedups by (user, book, started_at); pre-check now covers edition sessions too.
  • User-book progress and guest merge checked — not affected (no client-clock gate / same clock).
  • Web session queue left to fix(web): replay offline highlights; reading-session queue race #694.

Backward compat

Request/response shapes unchanged; old rows null → first write accepted; old mobile builds (send-time stamps) still correct.

Verified

UnitTests 1476 (10 new ProgressClockTests); mobile vitest 458 (5 new); shared vitest 560 (18 new); web hooks 152; tsc clean everywhere; dotnet format clean. Integration: the old "known defect" test inverted to MarkFinished_DeviceClockBehindTheServer_NewerWriteStillWins + new stale-write test — run by CI docker job.

Rollback

Revert (column is additive; leave it).

🤖 Generated with Claude Code

@mrviduus
mrviduus enabled auto-merge (squash) October 4, 2026 20:01
@mrviduus
mrviduus disabled auto-merge October 4, 2026 20:07
@mrviduus
mrviduus force-pushed the fix/progress-clock-and-sessions branch 2 times, most recently from b64e617 to 6a9b123 Compare October 4, 2026 21:17
mrviduus and others added 2 commits October 4, 2026 17:41
…ries failed reading sessions

- reading_progresses.client_updated_at: catalog progress stale-write guard compared client
  UpdatedAt with server-stamped UpdatedAt, so a device clock behind the server had newer writes
  refused with 200. Now compared only with the stored client stamp (ProgressClock), clamped to
  server now + 5 min so a future clock can't freeze the row.
- mobile stamps progress with the time it was recorded (snapshot), like web.
- mobile: failed session submits queued in AsyncStorage (cap 50), flushed after each session,
  on sign-in/app start and on reconnect; cleared on sign-out. Queue rules shared with web.
- server: session dedup pre-check covers edition sessions too (unique index already existed).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mrviduus
mrviduus force-pushed the fix/progress-clock-and-sessions branch from 6a9b123 to 8de3efb Compare October 4, 2026 21:42
@mrviduus
mrviduus merged commit b50ad98 into main Oct 4, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant