Skip to content

fix: store lastUpdate on the CRDT session record for reliable recovery#911

Open
Anexus5919 wants to merge 1 commit into
Somil450:mainfrom
Anexus5919:fix/crdt-list-sessions-decode
Open

fix: store lastUpdate on the CRDT session record for reliable recovery#911
Anexus5919 wants to merge 1 commit into
Somil450:mainfrom
Anexus5919:fix/crdt-list-sessions-decode

Conversation

@Anexus5919

Copy link
Copy Markdown
Contributor

📌 Related Issue

Fixes #874


📝 Description

listActiveSessions in src/services/crdtSessionEngine.ts rebuilt a Y.Doc and decoded a Yjs update per session synchronously on the main thread (during the mount cursor walk) just to read lastUpdate. It also applied only the last persisted delta, which for a non-compacted session does not carry the lastUpdate field, so lastUpdate read as 0, and the 30-minute recency filter in useWorkoutSync dropped the session, silently breaking active-session recovery on reload.

🔹 What has been changed?

  • persistUpdate: write lastUpdate as a plain field on the IndexedDB record.
  • listActiveSessions: read value.lastUpdate directly, with no Y.Doc construction and no decode.

🔹 Why are these changes needed?

  • Reading lastUpdate from a single partial delta is incorrect (the field usually isn't in that delta, so it reads 0) and also pays a per-session main-thread Yjs decode on mount. Storing lastUpdate as a plain field makes the recency read correct and removes the decode. loadSessionFromDB (the real recovery path) already applies all updates and is unaffected; pre-existing records lack the field and age out within the 24h cleanup window.

🛠️ Type of Change

  • 🐛 Bug Fix

🧪 Testing

✅ Tests Performed

  • Tested locally
  • npx tsc --noEmit: changed file clean; npx eslint: 0 problems.
  • Verified by reasoning about the Yjs delta semantics (a single non-compacted delta does not carry the lastUpdate key) and the useWorkoutSync recency filter.

🔹 Note on automated tests

No committed unit test: this path depends on IndexedDB and the repo has no fake-indexeddb test harness, so adding one would drag in unrelated lockfile changes. Verification is by tsc/eslint and the reasoning above.

🌐 Browsers Tested

Not applicable (IndexedDB/sync logic).


📷 Screenshots / Demo (if applicable)

Not applicable.


📋 Checklist

  • I have read the project's CONTRIBUTING guidelines
  • My code follows the project style guidelines
  • I have performed a self-review of my code
  • I have tested my changes locally
  • I have added/updated documentation where necessary (not applicable)
  • My changes do not introduce new warnings or errors
  • This PR is linked to an existing issue

💬 Additional Notes

Branched from latest upstream/main (5464425). The original finding also mentioned a "lost-update race"; I investigated and it does not hold (persistUpdate runs its get/put in the same transaction, and IndexedDB serializes overlapping readwrite transactions, so concurrent persists cannot read stale data). The decode/recovery bug above is the real, reachable issue, and that is what this PR fixes.

@vercel

vercel Bot commented Jun 22, 2026

Copy link
Copy Markdown

@Anexus5919 is attempting to deploy a commit to the somiljain2024-4175's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Anexus5919

Copy link
Copy Markdown
Contributor Author

@Somil450 @diksha78dev Kindly have a review on this pr. Thanks!

listActiveSessions built a Y.Doc and decoded a Yjs update per session on
the main thread just to read lastUpdate, and it applied only the last
partial delta. For a non-compacted session that delta does not carry the
lastUpdate field, so it read as 0 and the 30-minute recency filter in
useWorkoutSync dropped the session, breaking active-session recovery on
reload.

Persist lastUpdate as a plain field on the record and read it directly,
removing the per-session main-thread decode and fixing the recency read.
@Anexus5919 Anexus5919 force-pushed the fix/crdt-list-sessions-decode branch from 15eab14 to 679723e Compare June 23, 2026 17:34
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.

listActiveSessions decodes a Yjs doc per session on the main thread and reads lastUpdate=0, breaking session recovery

1 participant