Repository navigation
fix(sync): prefer agreed base over misleading file mtimes - #42
Merged
Omar-VF merged 3 commits intoOct 15, 2026
Merged
Conversation
added 3 commits
October 9, 2026 10:33
Compute ranked differing files by mtime before asking whether one whole manifest was still the confirmed common version. A restored or touched copy of that base could then overwrite the only real edit. Named extra locations called Compute with an empty base, so they had the same hole. A missing or stale base is unchanged. Issue Liquid-co#41; not the version-vector work in Liquid-co#29/Liquid-co#30.
The first daemon test only pushed a local edit over a newer-mtime base. This also pulls a remote edit that is older on disk, on the primary folder and a named root, and checks a lineage deletion is not pulled back.
|
Someone is attempting to deploy a commit to the sivadaboi's projects Team on Vercel. A member of the Team first needs to authorize it. |
Collaborator
|
Thanks, and for #41. We reproduced the rollback on 2.4.1 with your test, and CI is green against |
Collaborator
|
Merged into |
This was referenced Oct 15, 2026
ChakraFusion
added a commit
to ChakraFusion/OpenSave
that referenced
this pull request
Oct 16, 2026
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]>
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.
Refs #41.
When both peers have a confirmed common manifest, and exactly one side later changes, that change should win. A touched, copied, or restored copy of the old save can have a newer file mtime and is not an edit.
This is a source-level correction. It is not a reconstruction of any particular historical incident.
What changes
ComputeWithDeletionsused to rank differing files byMtimeMsbefore asking whether one whole manifest was still the agreed hash. The newer timestamp could then overwrite the only real edit, with no conflict.If one side's whole manifest still equals the confirmed base and the other does not, the changed side wins, in either direction. Named extra locations now pass the same agreed hash into that decision. They previously computed direction with an empty base.
A recorded deletion is not undone by a later mtime on the surviving copy. Identical contents do not transfer. If both sides differ from the base, the existing conflict path is unchanged.
What this does not do
A missing or stale agreed hash still uses the old mtime fallback. That can still be wrong. This PR does not add version vectors, and it does not replace the design in #29 / draft #30.
#30 decides sync from save versions and touches a much wider set of files, including
internal/p2p/syncengine/version.goand migrations0038/0039. This branch only changes the existing agreed-hash comparison indecision.goandmultiroot.go. The two can be reviewed separately. If both land, the maintainer should choose which decision model remains.Tests
Red on unmodified
346d9bda:TestBaseFirstPreventsMtimeRollbackfollowed the newer mtime (incorrect push and pull). Green on this branch.Also covered: both directions on real isolated daemons for the primary folder and a named extra root; symmetric deletion; equal contents; no base; stale base; both sides changed.
go test -count=1 -timeout 25m ./e2e/...onaee1c1ago test -racefor the agreed-base filters-count=3on the rollback and daemon-direction testsgo vet,gofmt,git diff --checkaee1c1aToolchain: Linux only. The full earlier suite used
go1.27.1-X:nodwarf5.go.modrequests Go 1.26.4. This tip was not rebuilt as a full E2E run on 1.26.4 because the commit is unchanged from that green run. No Windows or macOS run.TestSessionNamesTheSnapshotAlreadyTakenfails on an untouched archive of346d9bda(extraAuto backupsnapshot;want the watcher). This branch does not change session code. The timestamp package gate did not include./internal/daemonfor that reason.go test ./...also cannot buildcmd/opensave-apphere: it embedsfrontend/dist, which is not in the clone. Neither was stubbed.