fix(cloud-sync): prevent stale snapshots from overwriting newer account state#561
Open
Ridanshi wants to merge 1 commit into
Open
fix(cloud-sync): prevent stale snapshots from overwriting newer account state#561Ridanshi wants to merge 1 commit into
Ridanshi wants to merge 1 commit into
Conversation
…nt state
hydrateFromCloud previously merged accounts as { ...local, ...cloud },
letting the Appwrite snapshot unconditionally overwrite every local field.
A device that earned tokens while offline and reconnected before the
offline queue was flushed would silently lose those balances.
Fix: DB.set now stamps every local account write with _syncTs (epoch ms).
sanitizeAccount propagates _syncTs into Appwrite so both sides carry the
same timestamp field. hydrateFromCloud reads both timestamps and picks the
authoritative side before merging:
localTs > cloudTs → local wins; Appwrite is re-pushed to catch up.
cloudTs >= localTs → cloud wins (standard cloud-wins merge).
Safety net for legacy records without _syncTs: Math.max is applied to
tokens and staked so an offline-earned balance is never silently discarded.
Covers: offline device recovery, reconnect, multi-device divergence.
Tests: scripts/test-cloud-sync-merge.mjs — 22 assertions, all passing.
Closes Shruti070107#543
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #543
Problem
hydrateFromCloudmerged accounts as{ ...local, ...cloud }, letting the Appwrite snapshot overwrite every local field unconditionally. A device that earned tokens while offline and reconnected before the offline queue was flushed would silently lose those balances — stale cloud state won regardless of which copy was more recent.Root cause
No freshness metadata existed on local or remote account records, so there was no way to determine which side of a merge was authoritative. The merge always applied cloud data on top of local, making the scenario in #543 reproducible any time:
hydrateFromCloudruns.Fix
Timestamp field (
_syncTs)DB.setnow stamps every local account write with_syncTs: Date.now().sanitizeAccountincludes_syncTsin every Appwrite push so both sides carry the same timestamp field over time.Freshness-aware merge in
hydrateFromCloudThis covers:
Safety net for legacy records
Accounts written before this fix carry no
_syncTs. When both sides have_syncTs === 0,Math.maxis applied totokensandstakedso an offline-earned balance is never silently discarded even on legacy data.Files changed
src/app.jsDB.setstamps_syncTson every account write; passes stamped object topushAccountand offline queuesrc/cloud-sync.jssanitizeAccountpersists_syncTs;hydrateFromCloudreplaced with timestamp-aware merge + re-push + legacy safety netscripts/test-cloud-sync-merge.mjsTest results
Covered scenarios:
5–6. Legacy records —
Math.maxsafety net fortokensandstaked8–9. Re-push triggered only when local is newer
10–11. Non-financial fields follow the winning side
_syncTspreserved in merged result