Repository navigation
sync: two devices pushing to each other is a conflict, not a loop - #78
Merged
Merged
Conversation
Since Liquid-co#42 a side whose manifest equals the agreed base counts as unchanged. When the same bytes come back while one device's base is still a round behind, that device reads the other's new save as "unchanged" and pushes; the other correctly sees itself as newer and pushes too. Neither pulls, nothing moves either base, and they ask each other to pull several times a second while the change never arrives (Liquid-co#71). The rule stays; the loop is caught. A peer asks this device to pull (NoteTriggered, from the LAN and relay trigger handlers), and this device would answer with a push about the very same two states it pushed about last time: the peer was told to pull and pushed back instead, with neither side changed. After pushLoopRounds (2) such answers the sync raises a conflict through the existing path, so both saves stay as they are, the user decides, and no further trigger goes out. Anything other than a push (a pull, deletions, agreement), or either side's content changing, ends the watch; this device repeating its own push without the peer asking is a retry and never counts. TestPushLoop_TwoDevicesPushingToEachOtherIsAConflict is Liquid-co#71's reproduction and fails without the check; TestPushLoop_RetryingOnItsOwnIsNoLoop and TestPushLoop_APeerThatChangedStartsOver cover what must not count. Liquid-co#42's rollback tests are unchanged. TestReportedWAN_AlternatingEditsMustNotConflict wrote the same "a-turn" and "b-turn" every round, which made it this case by accident; each turn now writes something new, so it tests alternating edits again. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@ChakraFusion is attempting to deploy a commit to the sivadaboi's projects Team on Vercel. A member of the Team first needs to authorize it. |
This was referenced Oct 16, 2026
Open
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.
Fixes #71 as Seagulls proposed there (option 2): the loop is caught, and #42's base-first rule stays as it is.
The loop. Since #42, a side whose manifest equals the agreed base counts as unchanged. When the same bytes come back while one device's base is still a round behind, that device reads the other's new save as "unchanged" and pushes. The other device correctly sees itself as newer and pushes too. Neither pulls, so nothing moves either base. They ask each other to pull several times a second, and the change never arrives.
The fix, following Seagulls' four points:
NoteTriggered). A sync with that peer that decides "push only", about exactly the same two states as this device's last push to it, means the peer was told to pull and pushed back, with neither side changed.pushLoopRounds), the sync raises a conflict through the existing path. Both saves stay as they are, and no further trigger goes out. An open conflict already blocks further syncs of that game, so the peer's own triggers can't restart the loop.Why two answers and not one: a peer can decide to push while this device's trigger is still on its way. The loop runs several times a second, so waiting for a second answer costs nothing.
Tests (
syncengine/pushloop_test.go):TestPushLoop_TwoDevicesPushingToEachOtherIsAConflict: Two devices push to each other forever when a save returns to an earlier state (since #42) #71's deterministic reproduction. Local "a-turn", the peer's "b-turn", and the base equal to the peer's state, so fix(sync): prefer agreed base over misleading file mtimes #42's rule pushes. The peer answers each push. It fails without the check (still pushing after two answers) and passes with it: a conflict, both saves untouched, and no third trigger.TestPushLoop_RetryingOnItsOwnIsNoLoopandTestPushLoop_APeerThatChangedStartsOver: what must not count.base_first_rollback_test.go) are unchanged and pass.One test changed on purpose.
TestReportedWAN_AlternatingEditsMustNotConflictwrote the same "a-turn" and "b-turn" every round, which made it this ABA case by accident. That is why it failed about 1 in 15 on Windows (#337, #347, #357). Each turn now writes something new, so it tests alternating edits again. The ABA case is covered by the engine tests above, where it is deterministic.What users will see: if a save is set back to bytes it had before while one device is a round behind (a setting toggled back, for example), they get a conflict to resolve instead of a sync that silently never finishes. As Seagulls noted on #71, save versions (#29) would remove the ambiguity itself. This is the focused safety fix until then.
Checks: rebased onto today's
dev-public: the unit tests and the related e2e tests pass locally, and fork CI (the full suite, Linux and Windows) is green.🤖 Generated with Claude Code