Skip to content

sync: deletions in extra locations survive a lost report or a late refresh - #49

Open
ChakraFusion wants to merge 3 commits into
Liquid-co:dev-publicfrom
ChakraFusion:pr/location-deletion-lost-report
Open

ChakraFusion wants to merge 3 commits into
Liquid-co:dev-publicfrom
ChakraFusion:pr/location-deletion-lost-report

Conversation

@ChakraFusion

Copy link
Copy Markdown

Part of #21. Found while looking into the CI flake on #23.

Two commits, one for each way a file deleted in an extra save location came back. Each comes with a test that fails without it.

sync: a deletion in an extra location survives a lost sync-complete report

The report that a peer took a location is also what records which files
both sides hold there. When it was lost - it times out on a loaded machine
("could not tell ... that ... is sync-complete: context deadline
exceeded") - a file deleted here afterwards read as one the peer had and
this device never did, and the next sync fetched it back: the deletion
undone. Seen on CI as TestMultiRootScenario_DeletionInAnExtraLocationPropagates
failing once on Windows (#23); reproduced deterministically
by TestMultiRootScenario_ADeletionSurvivesALostReport.

  • The existing repair - a push is proven once the peer holds exactly what
    was handed over - now runs before anything is classified, and restores
    the lineage too: both held exactly the pushed state, so its files are
    what was shared.
  • Extra locations use the deletions this device recorded, as the main
    folder does (ComputeWithDeletions): a recorded deletion does not depend
    on the lineage, and is acted on only while the peer's copy is still
    exactly what was deleted.

sync: an extra location's record keeps deletions in flight, as the main folder's does

The refresh a peer's sync-complete report starts runs in the background,
and rebuilt the record of which files both sides hold in an extra location
from this device's files as they were when it ran. A file deleted here in
the moment before - a loaded machine is enough - dropped out of the record,
read as one the peer had and this device never did, and the next sync
fetched it back. The second way TestMultiRootScenario_DeletionInAnExtraLocationPropagates
failed on Windows (#23, and again with the previous
commit, without a lost report); reproduced deterministically by
TestMultiRootScenario_ADeletionSurvivesALateRefresh.

persistRootLineage now does what persistLineage does for the main folder:
keeps previously recorded paths that one side still holds (deletions in
flight, keepPendingDeletions), and keeps excluded paths out.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Oct 13, 2026

Copy link
Copy Markdown

@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.

@ChakraFusion

Copy link
Copy Markdown
Author

The failing job is TestSoak_ManyAlternatingRoundsNeverConflict on Windows: one save folder, synced over the relay. This PR only changes code that runs for extra save locations (syncOneRoot, persistRootLineage), which that test doesn't use, and the test passed on every run of this branch on my fork. So it's unrelated to this PR, and a re-run of the failed job should pass.

I looked into why it failed anyway, and it's a real (rare) false conflict in the main sync path:

  • In round 7, A pulled B's edit. Round 8 started right away, so A edited again within ~3 seconds.
  • B only advances its merge base once it hears A holds exactly what B handed over. It learns that from the sync-complete report by fetching A's manifest in the background (RefreshLineage). By then A had already moved on, so the base was never advanced, and the push-record repair can't fire for the same reason.
  • B's next sync saw both sides differ from the old base: sync conflict on "soak" with "Soak-A": both sides modified since last sync. From then on B took nothing, and round 8 never arrived.

The same can happen outside tests when a game saves right after a sync. I'm preparing a small separate PR: after a pure pull, the puller tells the other device which state it now holds, and that device records it as the base when it is exactly what it handed over. I'll link it here once it's through CI.

@ChakraFusion

Copy link
Copy Markdown
Author

The fix for that false conflict is #51. CI is green on my fork; on this repo, one Windows e2e job failed in TestFreshRestore_SnapshotModeReportsWhatItImported ("daemon never became ready"), where a single CLI daemon fails to start. That's before any syncing, so it's unrelated to the change.

While chasing another CI timeout I also found #50: a relay request whose reply the relay dropped waited out the sync's whole 30-minute deadline instead of failing after 30 s.

@ChakraFusion

Copy link
Copy Markdown
Author

I added a third commit here after finding a flaw in this PR's repair. When an extra location is emptied, the game is held until someone answers. "Put them back" with no snapshot works by taking the location's files out of its shared-files record, so the next sync fetches them from the other device. The push-proof repair from the first commit wrote them back into that record, so they read as deleted on this device, and the deletions went to the peer: the location ended up empty on both devices. The repair now leaves the record alone while the game is held as emptied.

TestMultiRootScenario_PutBackOfAnEmptiedLocationSurvivesALostReport fails 3/3 without the change and passes with it, and this PR's other tests still pass. I found it while checking the same repair for the main folder, which had the same flaw. That fix is in a separate PR, since it builds on a different change.

ChakraFusion and others added 3 commits October 15, 2026 14:25
…eport

The report that a peer took a location is also what records which files
both sides hold there. When it was lost - it times out on a loaded machine
("could not tell ... that ... is sync-complete: context deadline
exceeded") - a file deleted here afterwards read as one the peer had and
this device never did, and the next sync fetched it back: the deletion
undone. Seen on CI as TestMultiRootScenario_DeletionInAnExtraLocationPropagates
failing once on Windows (Liquid-co#23); reproduced deterministically
by TestMultiRootScenario_ADeletionSurvivesALostReport.

- The existing repair - a push is proven once the peer holds exactly what
  was handed over - now runs before anything is classified, and restores
  the lineage too: both held exactly the pushed state, so its files are
  what was shared.
- Extra locations use the deletions this device recorded, as the main
  folder does (ComputeWithDeletions): a recorded deletion does not depend
  on the lineage, and is acted on only while the peer's copy is still
  exactly what was deleted.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…in folder's does

The refresh a peer's sync-complete report starts runs in the background,
and rebuilt the record of which files both sides hold in an extra location
from this device's files as they were when it ran. A file deleted here in
the moment before - a loaded machine is enough - dropped out of the record,
read as one the peer had and this device never did, and the next sync
fetched it back. The second way TestMultiRootScenario_DeletionInAnExtraLocationPropagates
failed on Windows (Liquid-co#23, and again with the previous
commit, without a lost report); reproduced deterministically by
TestMultiRootScenario_ADeletionSurvivesALateRefresh.

persistRootLineage now does what persistLineage does for the main folder:
keeps previously recorded paths that one side still holds (deletions in
flight, keepPendingDeletions), and keeps excluded paths out.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The push-proof repair for extra locations (first commit here) records the
pushed state's files as shared. While the game is held as emptied that
record is kept on purpose: "put them back" with no snapshot takes the
location's files out of it so the next sync fetches them from the peer.
Written back by the repair, they read as deleted on this device instead,
the deletions went to the peer, and both devices lost the location's files.

The repair now leaves the lineage alone while the game is held as emptied.
Found while checking the same repair for the main folder, which had the
same flaw.

Test: TestMultiRootScenario_PutBackOfAnEmptiedLocationSurvivesALostReport
(fails 3/3 without this: the location empty on both devices).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@ChakraFusion
ChakraFusion force-pushed the pr/location-deletion-lost-report branch from e887eca to 6df6bcd Compare October 15, 2026 12:40
@ChakraFusion

Copy link
Copy Markdown
Author

Rebased onto today's dev-public. The conflict was with #42 in syncOneRoot: it keeps #42's base (and its rules translation) and passes this PR's recorded deletions into the decision (ComputeWithDeletions(..., base, deleted)). The repair runs once, with the emptied-hold check. Unit tests and the multi-location and emptied-folder e2e tests pass locally.

This branch has not been deployed

No deployments
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.

1 participant