Fix viewer join broadcasting device registry DELETE changesets#366
Closed
cursor[bot] wants to merge 1 commit into
Closed
Fix viewer join broadcasting device registry DELETE changesets#366cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
When a desktop connects to a remote brain as viewer, clearClusterRegistryForViewerJoin wiped local devices/sync_cluster_state and flushLocalChanges relayed those DELETEs to the brain, which could tombstone the entire paired device registry cluster-wide. Discard unpublished crsql_changes for those tables after the local clear and reset the peer outbound cursor before connecting so only post-handshake updates (e.g. touchLocalDevice) are synced. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
|
Folded this finding into the active |
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.
Bug and impact
Cluster-wide device registry corruption (data loss)
When a desktop connects to a remote brain via
sync.connectToBrain,clearClusterRegistryForViewerJoindeletes all rows in the CRR-backeddevicesandsync_cluster_statetables locally, thenflushLocalChangespushes those DELETE changesets to the brain. The brain applies them viaapplyChanges, which can tombstone every paired device for the project cluster-wide. The same pending DELETEs can reach iOS peers when the machine becomes local brain again afterdisconnectFromBrain.Concrete trigger: Machine A is the sync brain with phone + desktop B registered. Desktop B connects to A as viewer → local registry clear → successful connect →
flushLocalChanges→ brain merges mass DELETEs → paired devices disappear from sync status.Root cause
devices/sync_cluster_stateare CRR tables; SQLDELETEgeneratescrsql_changesrows for the local site.disconnectFromBrainuses the same clear path without isolating changes from relay.Fix
AdeDb.sync.discardUnpublishedChangesForTables()and call it fromclearClusterRegistryForViewerJoinfordevicesandsync_cluster_stateso local clears are not relayed.syncPeerService.acknowledgeLocalDbVersion()before connecting so any stray version gap cannot replay discarded batches.Validation
npm run test -- src/main/services/sync/deviceRegistryService.test.ts -t "viewer join clear"(runs when cr-sqlite is available; skipped on Linux CI without the extension)Does not overlap open PRs #363 (CRR alter / remote switch binding), #364 (orchestration approval bypass), or #365 (orchestration stale manifest).