From 0b8745a766c25d4555f907bf0030b07248c25396 Mon Sep 17 00:00:00 2001 From: ChakraFusion <112588066+ChakraFusion@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:45:58 +0200 Subject: [PATCH 1/2] sync: untracking a game forgets what was served for it Served states (served.go) are kept in memory per game and peer and were not cleared on untrack, unlike everything else the game agreed with its peers (store.ForgetGameSyncState). A game tracked again within the hour could then bank a state served before the untrack as agreed. Both untrack paths, by hand and from a peer, now clear them too. (Asked for on #66.) Test: TestForgetServed_ForgetsOnlyThatGame. Co-Authored-By: Claude Opus 5.5 --- internal/daemon/daemon.go | 2 ++ internal/p2p/syncengine/served.go | 18 ++++++++++ internal/p2p/syncengine/served_forget_test.go | 34 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 internal/p2p/syncengine/served_forget_test.go diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 3d732f86..a053ccc8 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1081,6 +1081,7 @@ func (d *Daemon) UntrackGame(gameID string) error { // same ID and would inherit all of it — and a stale lineage reads a file // that went missing while untracked as a deletion to propagate. _ = d.Store.ForgetGameSyncState(gameID) + d.P2P.Sync.ForgetServed(gameID) d.P2P.ClearPendingResync(gameID) d.P2P.NotifyUntrack(gameID) return nil @@ -1238,6 +1239,7 @@ func (d *Daemon) untrackFromPeer(gameID string) { // And everything it agreed with any peer, for the reason given in // UntrackGame: the ID comes back, and this must not come back with it. _ = d.Store.ForgetGameSyncState(gameID) + d.P2P.Sync.ForgetServed(gameID) d.P2P.ClearPendingResync(gameID) d.Log.Log("info", fmt.Sprintf("game %q untracked on a paired device", gameID)) } diff --git a/internal/p2p/syncengine/served.go b/internal/p2p/syncengine/served.go index 43a0003c..a732eb20 100644 --- a/internal/p2p/syncengine/served.go +++ b/internal/p2p/syncengine/served.go @@ -1,6 +1,7 @@ package syncengine import ( + "strings" "time" "github.com/opensave/opensave/internal/delta" @@ -71,6 +72,23 @@ func (e *Engine) NoteServed(gameID, peerID string, m delta.Manifest) { e.served[key] = list } +// ForgetServed drops every served state recorded for a game, with any peer. +// Called when the game is untracked, with the rest of what it agreed with its +// peers (store.ForgetGameSyncState). The records live in memory, so they +// outlived the untrack: a game tracked again within the hour could bank a +// state served before it was untracked as agreed, against a base that the +// untrack had wiped. +func (e *Engine) ForgetServed(gameID string) { + prefix := servedKey(gameID, "") + e.servedMu.Lock() + defer e.servedMu.Unlock() + for key := range e.served { + if strings.HasPrefix(key, prefix) { + delete(e.served, key) + } + } +} + // servedUnderBase reports whether this device served hash to the peer while // base was the agreed state — and so whether a peer holding hash holds a state // this device held since that agreement. diff --git a/internal/p2p/syncengine/served_forget_test.go b/internal/p2p/syncengine/served_forget_test.go new file mode 100644 index 00000000..7fb7543a --- /dev/null +++ b/internal/p2p/syncengine/served_forget_test.go @@ -0,0 +1,34 @@ +package syncengine + +import ( + "testing" + + "github.com/opensave/opensave/internal/delta" +) + +// Untracking a game forgets what this device served for it, so a re-track +// cannot bank a state served before the untrack; other games keep theirs. +func TestForgetServed_ForgetsOnlyThatGame(t *testing.T) { + env := setupEngine(t) + write(t, env.localDir, "save.dat", "v1") + m, err := delta.BuildManifest(env.localDir) + if err != nil { + t.Fatal(err) + } + e := env.engine + e.NoteServed("game1", env.peer.ID, m) + e.NoteServed("game10", env.peer.ID, m) + base := e.Store.GetAgreedHash("game1", env.peer.ID) + if !e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) { + t.Fatal("setup: the served state was not recorded") + } + + e.ForgetServed("game1") + + if e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) { + t.Error("a state served before the untrack is still remembered") + } + if !e.servedUnderBase("game10", env.peer.ID, m.ManifestHash(), e.Store.GetAgreedHash("game10", env.peer.ID)) { + t.Error("forgetting game1 also forgot game10") + } +} From 071bf0b8c9049ae6d96dfeb100aa4756d6265863 Mon Sep 17 00:00:00 2001 From: ChakraFusion <112588066+ChakraFusion@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:54:23 +0200 Subject: [PATCH 2/2] sync: the served-manifest proof repairs the shared-files record too When a peer is found holding a state this device served it (served.go), the base is banked as agreed, but the record of which files both hold was left as it was, as the push proof did before #65. A file the peer took from what was served and this device deleted before its watcher noted it then read as one the peer had and this device never did, and was fetched back. The served proof now records the served state's files as shared too, except while the game is held as emptied (as in #65). (Asked for on #66.) This needs the previous commit: with served records surviving an untrack, the repair banked a re-tracked game's files as shared and deleted the peer's copies (TestUntrack_LateLineageWriteMustNotSurviveIntoARetrack and TestUntrackThenRetrack_DoesNotInheritStaleLineage fail 3/3 without it). Test: TestDeletion_SurvivesALostReportWhenServed (fails 3/3 without this: the deleted file comes back on both devices). Co-Authored-By: Claude Opus 5.5 --- e2e/deletion_record_test.go | 62 +++++++++++++++++++++++++++++++ internal/p2p/syncengine/engine.go | 11 ++++++ 2 files changed, 73 insertions(+) diff --git a/e2e/deletion_record_test.go b/e2e/deletion_record_test.go index 6e93a11e..8a0dd9d1 100644 --- a/e2e/deletion_record_test.go +++ b/e2e/deletion_record_test.go @@ -203,3 +203,65 @@ func TestDeletion_SurvivesALostReport(t *testing.T) { t.Errorf("the deletion never reached B: %q", got) } } + +// The same with the proof from the other direction: B pulled what A served it +// and holds it still, while the report that records the files both hold is +// lost. A banks the served state as agreed; the record of shared files is +// repaired with it, so a file A deletes before its watcher notices is a +// deletion for B, not a file to fetch back (#66). +func TestDeletion_SurvivesALostReportWhenServed(t *testing.T) { + a, b, gameID := pairAndTrack(t, "Lost Report Served Deletion", map[string]string{"seed.sav": "seed"}) + testutil.SettleSync(t, gameID, a, b) + st := a.Daemon.Store + var oldBase string + if !testutil.WaitFor(30*time.Second, func() bool { + oldBase = st.GetAgreedHash(gameID, b.NodeID()) + return oldBase != "" + }) { + t.Fatal("setup: the devices never recorded that they agree") + } + + a.WriteSave("auto.sav", "v1") + time.Sleep(syncSettleWindow) + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + if !testutil.WaitFor(30*time.Second, func() bool { return b.ReadSave("auto.sav") == "v1" }) { + t.Fatal("setup: auto.sav never reached B") + } + testutil.SettleSync(t, gameID, a, b) + + handedOver, err := delta.BuildManifest(a.SaveDir) + if err != nil { + t.Fatal(err) + } + if !testutil.WaitFor(30*time.Second, func() bool { + return st.GetAgreedHash(gameID, b.NodeID()) == handedOver.ManifestHash() + }) { + t.Fatal("setup: the report never arrived") + } + // Undone as a lost report leaves it, with only the served state to go + // by: the old base (which clears the push record), the old record of + // shared files, and what A served B under that base. + if err := st.SetAgreedHash(gameID, b.NodeID(), oldBase); err != nil { + t.Fatal(err) + } + if st.GetPushedHash(gameID, b.NodeID()) != "" { + t.Fatal("setup: a push is still on record") + } + if err := st.SetSyncState(gameID, b.NodeID(), []string{"seed.sav"}, nil); err != nil { + t.Fatal(err) + } + a.Daemon.P2P.Sync.NoteServed(gameID, b.NodeID(), handedOver) + + if err := os.Remove(filepath.Join(a.SaveDir, "auto.sav")); err != nil { + t.Fatal(err) + } + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) // before the watcher records the deletion + time.Sleep(5 * time.Second) + + if got := a.ReadSave("auto.sav"); got != "" { + t.Errorf("the deleted file came back: %q", got) + } + if got := b.ReadSave("auto.sav"); got != "" { + t.Errorf("the deletion never reached B: %q", got) + } +} diff --git a/internal/p2p/syncengine/engine.go b/internal/p2p/syncengine/engine.go index 6c1c965b..7df1eda5 100644 --- a/internal/p2p/syncengine/engine.go +++ b/internal/p2p/syncengine/engine.go @@ -605,8 +605,19 @@ func (e *Engine) SyncWithPeer(ctx context.Context, gameID string, peer Peer) (Re // Matched on the peer's save as served, before this side's exclusion rules // filtered it, which is how it was recorded; banked in today's filtered // terms. Only against the base it was served under (served.go). + // + // The record of which files both hold is repaired with it, as the push + // proof above does and for the same reason: without it, a file the peer + // took from what was served and this device deleted before the watcher + // noted it read as one the peer had and this device never did, and was + // fetched back. And not while the game is held as emptied, for the same + // reason as above. Safe since untracking forgets what was served + // (ForgetServed): a re-tracked game no longer banks a state served before. if remoteHash := remoteData.Manifest.ManifestHash(); agreedHash != remoteHash && e.servedUnderBase(gameID, peer.ID, unfilteredRemote.ManifestHash(), storedBase) { + if _, held, err := e.Store.GetDeletionHold(gameID); err == nil && !held { + e.persistLineage(gameID, peer.ID, remoteData.Manifest, remoteData.Manifest) + } _ = e.Store.SetAgreedHash(gameID, peer.ID, remoteHash) agreedHash = remoteHash }