Skip to content

watcher: changes OpenSave makes itself are not new saves - #23

Merged
Omar-VF merged 1 commit into
Liquid-co:dev-publicfrom
ChakraFusion:pr/own-writes-not-saves
Oct 15, 2026
Merged

Omar-VF merged 1 commit into
Liquid-co:dev-publicfrom
ChakraFusion:pr/own-writes-not-saves

Conversation

@ChakraFusion

Copy link
Copy Markdown

Part of #21.

The bug

Every change a sync applies looks exactly like the game saving. This has two effects:

  • Snapshot floods. A peer deleting files one request at a time caused an auto-snapshot every couple of seconds. Each one held a save that was neither the old one nor the new one. They filled the retention budget and pushed out the snapshots that mattered.
  • Echo. OnChanged fired for pulled files, so the sync was sent straight back out to the peers.

The fix

A new small package, internal/owntouch, records the paths OpenSave itself touches:

  • pulled files (delta.PatchWriter);
  • the folders a pull creates;
  • a peer's deletions, and the files "keep theirs" removes;
  • the mtimes "keep mine" re-dates.

The watcher classifies each burst of events against it:

  • A burst made only of OpenSave's own changes moves the recorded hash and nothing else: no auto-snapshot and no OnChanged.
  • A file counts as OpenSave's only if nothing wrote it after the mark (its mtime is not later). Once OpenSave is done with a file (renamed into place, re-dated to the peer's time), it records the exact time and size it left the file with. From then on the file is OpenSave's only while both still match. A game saving right after a sync is therefore still the game's change, even within the 2 seconds a plain time comparison would have to allow.
  • A path found gone counts as OpenSave's only if OpenSave removed it (MarkRemoved). If the game or a person deletes a file OpenSave has just pulled, that deletion is theirs, and it is synced and snapshotted as such.
  • The rescan after new folders come under watch counts as OpenSave's own only when OpenSave created those folders, for example a pull of a save with subfolders.

Tests

  • owntouch_test.go:
    • a marked file and its folder count as ours, and a sibling doesn't;
    • a write after the mark is not ours;
    • a file deleted by someone else after OpenSave wrote it is not ours, while a file OpenSave removed is;
    • a same-size save written 20 ms after a finished pull is not ours.
  • watcher/owntouch_test.go: own changes and own new folders are not snapshotted, and a burst mixing own changes with the game's is.
  • go test ./... passes. CI on the fork (all jobs green): https://github.com/ChakraFusion/OpenSave/actions/runs/37174971888

🤖 Generated with Claude Code

New package internal/owntouch records the paths OpenSave itself writes,
removes, creates or re-dates: pulled files, a peer's deletions, the folders a
pull creates, the files "keep theirs" removes and the mtimes "keep mine"
touches. The watcher classifies every event by it: a burst made only of
OpenSave's own changes moves the recorded hash and nothing else - no
auto-snapshot, and no OnChanged sending the sync straight back out.

Before, every change a sync applied looked exactly like the game saving. A peer
deleting files one request at a time produced an auto-snapshot every couple of
seconds, each of a save that was neither the old one nor the new one; they
filled the retention budget and pushed out the snapshots that mattered.

- a file counts as OpenSave's only if nothing wrote it after the mark (its
  mtime is not later), so a game saving straight after a sync is still the
  game's change.
- once OpenSave is done with a file (renamed into place, re-dated to the
  peer's time) it records the time and size it left it with, and from then
  on the file is OpenSave's only while it has exactly those - a game saving
  within the two seconds the time comparison allows is still the game's.
- a path found gone counts as OpenSave's only if OpenSave removed it
  (MarkRemoved): a file it pulled and the game or a person then deleted is
  their change.
- the rescan after new folders come under watch is OpenSave's own when it
  created those folders (a pull of a save with subfolders), and someone
  else's otherwise.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@vercel

vercel Bot commented Oct 4, 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.

@Omar-VF

Omar-VF commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

CI is green now, but the first run failed once on Windows in TestMultiRootScenario_DeletionInAnExtraLocationPropagates: the deletion in the second location never reached the peer, and device A kept reporting "already in sync" after logging "file updated: keys.cfg". It passed on a re-run, and 10 out of 10 for us locally, on this branch and on main. Since the PR changes how a deletion is classified as OpenSave's own, could you check whether a user's deletion in an extra location could be misread as ours under slow timing? We'll merge once that's ruled out.

ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 13, 2026
…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]>
@ChakraFusion

Copy link
Copy Markdown
Author

Thanks for checking this so carefully. I looked into it, and it isn't the deletion being taken for OpenSave's own. It's a weakness in how extra locations record which files both devices hold, and it exists on dev-public without this PR.

In the failing run's log, device A did treat the deletion as the user's. Right after the test removed keys.cfg, A logged auto-snapshot created and a post-snapshot sync. If the deletion had been classified as OpenSave's own, neither would happen. Also, owntouch.Recent on a path that is gone only returns true when OpenSave itself removed it (MarkRemoved), and A never wrote or removed keys.cfg.

The deletion was undone because A's record of what both devices hold in that location (the per-location lineage) had lost keys.cfg. A then read the peer's copy as a file A never had and fetched it back. I found two ways that record goes wrong, both timing-dependent and both reproduced deterministically on dev-public:

  1. A lost sync-complete report. Your failing run shows could not tell LocDelete-A that locdelete is sync-complete: ... context deadline exceeded. Without the report, nothing records that the peer took the files. The existing "push proven by the peer holding it" repair restored the base hash, but not the lineage.
  2. A late refresh. When the report does arrive, it starts a background refresh that rebuilds the lineage from this device's files as they are when it runs. A file deleted just before that drops out. The main folder's persistLineage avoids this by keeping deletions in flight (keepPendingDeletions), but persistRootLineage didn't. (My first fix only covered path 1; CI on my fork then hit path 2.)

The fix is a separate small PR: #49. It has two commits and two new tests (TestMultiRootScenario_ADeletionSurvivesALostReport, …ALateRefresh), each failing without its fix:

  • the repair now runs before anything is classified, and restores the lineage too;
  • extra locations use recorded deletions (ComputeWithDeletions), as the main folder does;
  • persistRootLineage keeps deletions in flight and excluded paths out, as persistLineage does.

So I'd say #23 is fine to merge as it is, and the flake is addressed by the new PR.

ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 13, 2026
…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]>
ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 13, 2026
…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]>
ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 13, 2026
A pull of a save of many files overflowed the watcher's event queue again
and again, and each overflow was taken for the game saving: an automatic
snapshot of a half-pulled save every few seconds - 76 on one device for one
pull - which pushed the complete ones out of retention.

- The watcher asks whether a sync is writing the game (BeingWritten). If
  so, an overflow is taken for the sync's, and the burst is looked at only
  once the sync has finished.
- A snapshot that could not read every file is discarded, not kept: it
  would stand in the history as a save that never existed. The watcher
  leaves the change unrecorded and takes it again at its next pass; a sync
  that needed the copy before replacing files does not replace them.
- OpenSave's own temporary files (*.opensave.tmp) are never archived.

Builds on the own-writes change (Liquid-co#23).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Omar-VF
Omar-VF merged commit 411eb80 into Liquid-co:dev-public Oct 15, 2026
19 of 21 checks passed
ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 15, 2026
…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]>
ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 15, 2026
…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]>
ChakraFusion added a commit to ChakraFusion/OpenSave that referenced this pull request Oct 15, 2026
A pull of a save of many files overflowed the watcher's event queue again
and again, and each overflow was taken for the game saving: an automatic
snapshot of a half-pulled save every few seconds - 76 on one device for one
pull - which pushed the complete ones out of retention.

- The watcher asks whether a sync is writing the game (BeingWritten). If
  so, an overflow is taken for the sync's, and the burst is looked at only
  once the sync has finished.
- A snapshot that could not read every file is discarded, not kept: it
  would stand in the history as a save that never existed. The watcher
  leaves the change unrecorded and takes it again at its next pass; a sync
  that needed the copy before replacing files does not replace them.
- OpenSave's own temporary files (*.opensave.tmp) are never archived.

Builds on the own-writes change (Liquid-co#23).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.

2 participants