diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d5300ce..2347dc41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to OpenSave are documented here. This project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] + +### Added + +- **A game can be tracked or placed without syncing until it is released.** + `provisioningHold: true` (or `autoSync: false`) on `POST /api/games` and + `POST /api/offered-games/{id}/place` records the game and a hold in one + step. The hold survives a daemon restart and is not the ordinary AutoSync + flag: peers cannot pull or push that game's saves, and releasing one game + does not sync the rest of the library. `POST /api/games/{id}/release-provisioning` + clears the hold. An empty body turns AutoSync on, so reconcile can sync + that game with every online peer that is not still held. `{"autoSync": false}` + clears the hold and leaves AutoSync off, so reconcile, reconnect and the + watcher do not sync it; the caller syncs explicitly. A PATCH cannot clear + the hold. `provisioningHoldUnknown` is true when the hold cannot be read, + and `provisioningHold` is not reported false in that case. See issue #40. + ## [2.4.1] — 2026-10-02 A security update: please install it on every device. A paired device could diff --git a/e2e/provisioning_hold_test.go b/e2e/provisioning_hold_test.go new file mode 100644 index 00000000..8c3af641 --- /dev/null +++ b/e2e/provisioning_hold_test.go @@ -0,0 +1,737 @@ +package e2e + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "net/http" + "os" + "path/filepath" + "testing" + "time" + + "github.com/opensave/opensave/internal/p2p/syncengine" + "github.com/opensave/opensave/internal/store" + "github.com/opensave/opensave/testutil" +) + +func sideDir(t *testing.T, td *testutil.TestDaemon, name string) string { + t.Helper() + dir := filepath.Join(filepath.Dir(td.SaveDir), name) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + return dir +} + +func writeHoldFile(t *testing.T, dir, rel, content string) { + t.Helper() + full := filepath.Join(dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o600); err != nil { + t.Fatal(err) + } +} + +func readHoldFile(t *testing.T, dir, rel string) string { + t.Helper() + path := filepath.Join(dir, filepath.FromSlash(rel)) + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatalf("reading save fixture %q: %v", path, err) + } + return string(raw) +} + +func fileHash(dir, rel string) string { + raw, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(rel))) + if err != nil { + return "" + } + sum := sha256.Sum256(raw) + return hex.EncodeToString(sum[:]) +} + +type holdGame struct { + ID string `json:"id"` + AutoSync bool `json:"autoSync"` + ProvisioningHold bool `json:"provisioningHold"` + SavePath string `json:"savePath"` +} + +func createHeld(t *testing.T, td *testutil.TestDaemon, id, name, path string) holdGame { + t.Helper() + var got holdGame + td.API(http.MethodPost, "/api/games", map[string]any{ + "id": id, "name": name, "savePath": path, "provisioningHold": true, + }, &got) + if got.ID != id || got.AutoSync || !got.ProvisioningHold { + t.Fatalf("held create = %+v", got) + } + held, err := td.Daemon.Store.ProvisioningHeld(id) + if err != nil || !held { + t.Fatalf("hold not committed for %s: %v", id, err) + } + return got +} + +func assertHeldQuiet(t *testing.T, source, bridge *testutil.TestDaemon, id, sourceDir, bridgeDir, marker string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + source.Daemon.P2P.PingPairedPeers(ctx) + source.Daemon.P2P.SyncAllGames(ctx) + bridge.Daemon.P2P.PingPairedPeers(ctx) + bridge.Daemon.P2P.SyncAllGames(ctx) + if _, err := source.Daemon.P2P.Sync.SyncGame(ctx, id, source.Daemon.P2P.OnlinePeers()); !errors.Is(err, syncengine.ErrProvisioning) { + t.Fatalf("held game must refuse direct sync, got %v", err) + } + var syncResp struct { + Error string `json:"error"` + Reason string `json:"reason"` + } + status := source.APIStatus(http.MethodPost, "/api/games/"+id+"/sync", nil, &syncResp) + if status != http.StatusConflict || syncResp.Reason != "provisioning" { + t.Fatalf("direct sync of a held game returned %d %+v", status, syncResp) + } + if readHoldFile(t, bridgeDir, marker) != "" || readHoldFile(t, sourceDir, "from-bridge.sav") != "" { + t.Fatalf("held game transferred: bridge=%q source=%q", readHoldFile(t, bridgeDir, marker), readHoldFile(t, sourceDir, "from-bridge.sav")) + } + if _, err := bridge.Daemon.Store.GetGame(id); err != nil { + t.Fatal("bridge lost the held game") + } +} + +// A paired source can record a new game without the bridge learning it, and +// the bridge can record the same id itself. Restart, reconcile, a direct +// sync and the retry path must not move bytes. An unrelated game still syncs. +func TestProvisioningHold_PairedCreateRestartAndIsolation(t *testing.T) { + source := testutil.NewTestDaemon(t, "Hold-Source") + bridge := testutil.NewTestDaemon(t, "Hold-Bridge") + + unrelatedSrc := sideDir(t, source, "unrelated") + unrelatedBr := sideDir(t, bridge, "unrelated") + writeHoldFile(t, unrelatedSrc, "keep/unrelated.sav", "ONLY-UNRELATED") + writeHoldFile(t, unrelatedBr, "keep/unrelated.sav", "ONLY-UNRELATED") + source.API(http.MethodPost, "/api/games", map[string]string{"name": "Hold Unrelated", "savePath": unrelatedSrc}, nil) + bridge.API(http.MethodPost, "/api/games", map[string]string{"name": "Hold Unrelated", "savePath": unrelatedBr}, nil) + source.PairWith(bridge) + + newSrc := sideDir(t, source, "new-game") + newBr := sideDir(t, bridge, "new-game") + writeHoldFile(t, newSrc, "profile/source.sav", "ONLY-SOURCE") + const id = "hold-new-game" + createHeld(t, source, id, "Hold New Game", newSrc) + // No offer is required. The bridge is given the source's id directly. + offers, _ := bridge.Daemon.Store.ListOfferedGames() + if len(offers) != 0 { + t.Fatalf("held source published an offer: %+v", offers) + } + createHeld(t, bridge, id, "Hold New Game", newBr) + + writeHoldFile(t, unrelatedSrc, "keep/fresh.sav", "UNRELATED-FRESH") + if !testutil.WaitFor(30*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+store.SlugifyGameID("Hold Unrelated")+"/sync", nil, nil) + return readHoldFile(t, unrelatedBr, "keep/fresh.sav") == "UNRELATED-FRESH" + }) { + t.Fatal("unrelated game stopped syncing while another game was held") + } + if readHoldFile(t, newBr, "profile/source.sav") != "" { + t.Fatal("unrelated sync also copied the held game") + } + + // Crash after the row is committed: the next process is a fresh daemon.New. + source.Restart() + bridge.Restart() + for _, node := range []*testutil.TestDaemon{source, bridge} { + if node.Daemon.SyncPauseStatus().Paused { + t.Fatal("restart kept an in-memory pause; the hold must not depend on one") + } + held, err := node.Daemon.Store.ProvisioningHeld(id) + if err != nil || !held { + t.Fatalf("%s lost the hold across restart: %v", node.Name(), err) + } + } + assertHeldQuiet(t, source, bridge, id, newSrc, newBr, "profile/source.sav") + + var manifest struct { + Error string `json:"error"` + } + if status := bridge.APIStatus(http.MethodGet, "/api/p2p/manifest/"+id, nil, &manifest); status < 400 || manifest.Error != syncengine.ProvisioningMessage { + t.Fatalf("manifest of a held game = %d %q", status, manifest.Error) + } + if manifest.Error != "" && (len(manifest.Error) > 0 && containsNotFound(manifest.Error)) { + t.Fatalf("provisioning refusal looks like not-found: %q", manifest.Error) + } + writeHoldFile(t, newBr, "profile/bridge.sav", "ONLY-BRIDGE") + var deleted struct { + Error string `json:"error"` + } + if status := bridge.APIStatus(http.MethodPost, "/api/p2p/delete-file/"+id, map[string]string{"relPath": "profile/bridge.sav"}, &deleted); status < 400 { + t.Fatalf("delete-file on a held game returned %d", status) + } + if readHoldFile(t, newBr, "profile/bridge.sav") != "ONLY-BRIDGE" { + t.Fatal("inbound delete changed a held save") + } + + // One side released, then both processes replaced. The other hold remains. + var rel struct { + Released bool `json:"released"` + } + source.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, &rel) + if !rel.Released { + t.Fatal("source release did not clear the hold") + } + source.Restart() + bridge.Restart() + held, err := bridge.Daemon.Store.ProvisioningHeld(id) + if err != nil || !held { + t.Fatal("bridge hold did not survive a restart after only the source was released") + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + source.Daemon.P2P.PingPairedPeers(ctx) + source.Daemon.P2P.SyncAllGames(ctx) + if readHoldFile(t, newBr, "profile/source.sav") != "" || readHoldFile(t, newSrc, "profile/bridge.sav") != "" { + t.Fatal("one-sided release transferred saves") + } + + bridge.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, &rel) + if !testutil.WaitFor(45*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+id+"/sync", nil, nil) + return readHoldFile(t, newBr, "profile/source.sav") == "ONLY-SOURCE" && + fileHash(newSrc, "profile/source.sav") == fileHash(newBr, "profile/source.sav") + }) { + t.Fatalf("release did not sync: bridge=%q", readHoldFile(t, newBr, "profile/source.sav")) + } + if readHoldFile(t, unrelatedBr, "keep/fresh.sav") != "UNRELATED-FRESH" { + t.Fatal("release disturbed the unrelated game") + } +} + +func containsNotFound(s string) bool { + return len(s) >= 9 && (s == "Game not found." || (len(s) > 0 && (containsFold(s, "not found")))) +} + +func containsFold(s, sub string) bool { + return len(s) >= len(sub) && (s == sub || len(sub) == 0 || (func() bool { + for i := 0; i+len(sub) <= len(s); i++ { + if equalFoldASCII(s[i:i+len(sub)], sub) { + return true + } + } + return false + })()) +} + +func equalFoldASCII(a, b string) bool { + if len(a) != len(b) { + return false + } + for i := 0; i < len(a); i++ { + ca, cb := a[i], b[i] + if ca >= 'A' && ca <= 'Z' { + ca += 'a' - 'A' + } + if cb >= 'A' && cb <= 'Z' { + cb += 'a' - 'A' + } + if ca != cb { + return false + } + } + return true +} + +func TestProvisioningHold_PlaceOfferWithoutSyncing(t *testing.T) { + source := testutil.NewTestDaemon(t, "Place-Source") + bridge := testutil.NewTestDaemon(t, "Place-Bridge") + source.PairWith(bridge) + setUnknownGamePolicy(t, bridge, store.UnknownGameAsk) + + source.WriteSave("slot.sav", "FROM-SOURCE") + gameID := source.TrackGame("Place Held Game") + syncTo(source, gameID, bridge.NodeID()) + if !testutil.WaitFor(20*time.Second, func() bool { + offers, _ := bridge.Daemon.Store.ListOfferedGames() + return len(offers) == 1 && offers[0].GameID == gameID + }) { + t.Fatal("bridge did not record an offer to place") + } + + dest := sideDir(t, bridge, "placed") + var placed holdGame + bridge.API(http.MethodPost, "/api/offered-games/"+gameID+"/place", map[string]any{ + "path": dest, "autoSync": false, + }, &placed) + if placed.ID != gameID || !placed.ProvisioningHold || placed.AutoSync { + t.Fatalf("held place = %+v", placed) + } + bridge.API(http.MethodPost, "/api/offered-games/"+gameID+"/place", map[string]any{ + "path": dest, "provisioningHold": true, + }, &placed) + games, _ := bridge.Daemon.Store.ListGames() + n := 0 + for _, g := range games { + if g.ID == gameID { + n++ + } + } + if n != 1 { + t.Fatalf("repeated place created %d games", n) + } + source.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + if readHoldFile(t, dest, "slot.sav") != "" { + t.Fatal("placing with autoSync false synced the save") + } + + bridge.API(http.MethodPost, "/api/games/"+gameID+"/release-provisioning", nil, nil) + if !testutil.WaitFor(45*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + return readHoldFile(t, dest, "slot.sav") == "FROM-SOURCE" + }) { + t.Fatalf("release did not deliver the save, dest=%q", readHoldFile(t, dest, "slot.sav")) + } +} + +func TestProvisioningHold_RejectsUnsafePathsAndDuplicates(t *testing.T) { + node := testutil.NewTestDaemon(t, "Hold-Paths") + live := sideDir(t, node, "live") + node.API(http.MethodPost, "/api/games", map[string]string{"name": "Live Occupant", "savePath": live}, nil) + + var errBody struct { + Error string `json:"error"` + } + if status := node.APIStatus(http.MethodPost, "/api/games", map[string]any{ + "id": "other-id", "name": "Other", "savePath": live, "provisioningHold": true, + }, &errBody); status < 400 { + t.Fatal("held create accepted an occupied folder") + } + if g, err := node.Daemon.Store.GetGame("other-id"); err == nil { + t.Fatalf("occupied create still inserted %+v", g) + } + + outside := sideDir(t, node, "outside") + link := filepath.Join(filepath.Dir(node.SaveDir), "escape-link") + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + if status := node.APIStatus(http.MethodPost, "/api/games", map[string]any{ + "id": "symlink-game", "name": "Symlink", "savePath": link, "provisioningHold": true, + }, &errBody); status < 400 { + t.Fatal("held create accepted a symlink") + } + + primary := sideDir(t, node, "primary") + createHeld(t, node, "dup-game", "Dup", primary) + other := sideDir(t, node, "other") + if status := node.APIStatus(http.MethodPost, "/api/games", map[string]any{ + "id": "dup-game", "name": "Dup", "savePath": other, "provisioningHold": true, + }, &errBody); status < 400 { + t.Fatal("a second path for a held id was accepted") + } + again := createHeld(t, node, "dup-game", "Dup", primary) + if again.SavePath == "" { + t.Fatal("retry of the same held create failed") + } + if status := node.APIStatus(http.MethodPost, "/api/games", map[string]any{ + "id": "pinned", "name": "Pinned", "savePath": primary, + }, &errBody); status != http.StatusBadRequest { + t.Fatalf("explicit id without a hold returned %d", status) + } +} + +func TestProvisioningHold_NamedRootsBeforeRelease(t *testing.T) { + source := testutil.NewTestDaemon(t, "Roots-Source") + bridge := testutil.NewTestDaemon(t, "Roots-Bridge") + source.PairWith(bridge) + const id = "roots-game" + src := sideDir(t, source, "main") + br := sideDir(t, bridge, "main") + srcA, srcB := sideDir(t, source, "root-a"), sideDir(t, source, "root-b") + brA, brB := sideDir(t, bridge, "root-a"), sideDir(t, bridge, "root-b") + writeHoldFile(t, src, "main.sav", "MAIN") + writeHoldFile(t, srcA, "a.sav", "ROOT-A") + writeHoldFile(t, srcB, "b.sav", "ROOT-B") + createHeld(t, source, id, "Roots Game", src) + createHeld(t, bridge, id, "Roots Game", br) + for _, spec := range []struct { + node *testutil.TestDaemon + name, path string + }{ + {source, "config", srcA}, + {source, "mods", srcB}, + {bridge, "config", brA}, + {bridge, "mods", brB}, + } { + spec.node.API(http.MethodPost, "/api/games/"+id+"/roots", map[string]string{"name": spec.name, "path": spec.path}, nil) + } + if status := source.APIStatus(http.MethodPost, "/api/games/"+id+"/sync", nil, nil); status < 400 { + t.Fatalf("sync of a held game returned %d", status) + } + if readHoldFile(t, br, "main.sav") != "" || readHoldFile(t, brA, "a.sav") != "" || readHoldFile(t, brB, "b.sav") != "" { + t.Fatal("named roots synced before release") + } + source.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, nil) + bridge.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, nil) + if !testutil.WaitFor(45*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+id+"/sync", nil, nil) + return readHoldFile(t, br, "main.sav") == "MAIN" && readHoldFile(t, brA, "a.sav") == "ROOT-A" && readHoldFile(t, brB, "b.sav") == "ROOT-B" + }) { + t.Fatalf("roots after release: main=%q a=%q b=%q", readHoldFile(t, br, "main.sav"), readHoldFile(t, brA, "a.sav"), readHoldFile(t, brB, "b.sav")) + } +} + +func TestProvisioningHold_HundredGamesStayOneDaemon(t *testing.T) { + source := testutil.NewTestDaemon(t, "Bulk-Source") + receiver := testutil.NewTestDaemon(t, "Bulk-Receiver") + liveSrc := sideDir(t, source, "live") + liveRecv := sideDir(t, receiver, "live") + writeHoldFile(t, liveSrc, "live.sav", "LIVE") + source.API(http.MethodPost, "/api/games", map[string]string{"name": "Bulk Live", "savePath": liveSrc}, nil) + receiver.API(http.MethodPost, "/api/games", map[string]string{"name": "Bulk Live", "savePath": liveRecv}, nil) + source.PairWith(receiver) + liveID := store.SlugifyGameID("Bulk Live") + if !testutil.WaitFor(30*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+liveID+"/sync", nil, nil) + return readHoldFile(t, liveSrc, "live.sav") == "LIVE" && readHoldFile(t, liveRecv, "live.sav") == "LIVE" + }) { + t.Fatal("the unrelated live game did not establish an initial sync") + } + srcBase := source.Daemon.Store.GetAgreedHash(liveID, receiver.NodeID()) + recvBase := receiver.Daemon.Store.GetAgreedHash(liveID, source.NodeID()) + if srcBase == "" || recvBase == "" || srcBase != recvBase { + t.Fatalf("initial sync left no bilateral agreed base: source=%q receiver=%q", srcBase, recvBase) + } + + start := time.Now() + for i := 0; i < 100; i++ { + id := "bulk-" + hex.EncodeToString([]byte{byte(i)}) + dir := sideDir(t, source, id) + writeHoldFile(t, dir, "held.sav", "HELD-"+id) + createHeld(t, source, id, "Bulk "+id, dir) + } + if elapsed := time.Since(start); elapsed > 45*time.Second { + t.Fatalf("100 held creates took %s; this must not be a daemon per game", elapsed) + } + offers, err := receiver.Daemon.Store.ListOfferedGames() + if err != nil { + t.Fatal(err) + } + if len(offers) != 0 { + t.Fatalf("held creates published %d offers", len(offers)) + } + writeHoldFile(t, liveSrc, "live.sav", "LIVE-UPDATED") + if !testutil.WaitFor(30*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+liveID+"/sync", nil, nil) + return readHoldFile(t, liveRecv, "live.sav") == "LIVE-UPDATED" && readHoldFile(t, liveSrc, "live.sav") == "LIVE-UPDATED" + }) { + t.Fatal("the live game did not sync beside 100 held games") + } + if _, err := os.Stat(filepath.Join(liveRecv, "held.sav")); !os.IsNotExist(err) { + t.Fatalf("a held save appeared on the receiver: %v", err) + } + source.Restart() + set, err := source.Daemon.Store.ProvisioningHeldSet() + if err != nil { + t.Fatal(err) + } + if len(set) != 100 { + t.Fatalf("restart kept %d holds, want 100", len(set)) + } +} + +func TestProvisioningHold_DoesNotConvertAnExistingGame(t *testing.T) { + node := testutil.NewTestDaemon(t, "Hold-Existing") + var live holdGame + node.API(http.MethodPost, "/api/games", map[string]string{ + "name": "Already Live", "savePath": node.SaveDir, + }, &live) + writeHoldFile(t, node.SaveDir, "keep.sav", "LIVE-BYTES") + var errBody struct { + Error string `json:"error"` + } + if status := node.APIStatus(http.MethodPost, "/api/games", map[string]any{ + "id": live.ID, "name": "Already Live", "savePath": node.SaveDir, "provisioningHold": true, + }, &errBody); status < 400 { + t.Fatal("a hold was accepted for a game that was already tracking") + } + got, err := node.Daemon.Store.GetGame(live.ID) + if err != nil { + t.Fatal(err) + } + if !got.AutoSync { + t.Fatal("the failed hold turned AutoSync off") + } + held, err := node.Daemon.Store.ProvisioningHeld(live.ID) + if err != nil || held { + t.Fatalf("existing game held=%v err=%v", held, err) + } + if readHoldFile(t, node.SaveDir, "keep.sav") != "LIVE-BYTES" { + t.Fatal("the failed hold changed save bytes") + } +} + +// Three peers in a star: ends talk only to the middle. Releasing the two ends +// that should converge must not move the third while it is still held, and +// release itself must not start that transfer. +func TestProvisioningHold_StarLeavesHeldEndpointAlone(t *testing.T) { + leafA := testutil.NewTestDaemon(t, "Star-LeafA") + hub := testutil.NewTestDaemon(t, "Star-Hub") + leafB := testutil.NewTestDaemon(t, "Star-LeafB") + leafA.PairWith(hub) + leafB.PairWith(hub) + if _, err := leafA.Daemon.Store.GetPeer(leafB.NodeID()); err == nil { + t.Fatal("leafA is paired with leafB; the fixture is not a star") + } + + const id = "star-game" + bz := sideDir(t, leafA, "star") + br := sideDir(t, hub, "star") + dk := sideDir(t, leafB, "star") + writeHoldFile(t, bz, "progress.sav", "LEAF-A-ONLY") + writeHoldFile(t, dk, "progress.sav", "LEAF-B-ONLY") + createHeld(t, leafA, id, "Star Game", bz) + createHeld(t, hub, id, "Star Game", br) + createHeld(t, leafB, id, "Star Game", dk) + + var rel struct { + Released bool `json:"released"` + } + leafA.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, &rel) + hub.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, &rel) + // Default release enables AutoSync; the released pair may converge + // before this assertion. The held third peer must remain untouched. + if readHoldFile(t, dk, "progress.sav") != "LEAF-B-ONLY" { + t.Fatal("default release touched the held third peer") + } + if !testutil.WaitFor(45*time.Second, func() bool { + leafA.API(http.MethodPost, "/api/games/"+id+"/sync", nil, nil) + return readHoldFile(t, br, "progress.sav") == "LEAF-A-ONLY" && + readHoldFile(t, dk, "progress.sav") == "LEAF-B-ONLY" && + readHoldFile(t, bz, "progress.sav") == "LEAF-A-ONLY" + }) { + t.Fatalf("star sync moved a held endpoint or failed to converge the released pair: hub=%q leafB=%q leafA=%q", + readHoldFile(t, br, "progress.sav"), readHoldFile(t, dk, "progress.sav"), readHoldFile(t, bz, "progress.sav")) + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + hub.Daemon.P2P.PingPairedPeers(ctx) + hub.Daemon.P2P.SyncAllGames(ctx) + if readHoldFile(t, dk, "progress.sav") != "LEAF-B-ONLY" || readHoldFile(t, bz, "progress.sav") != "LEAF-A-ONLY" { + t.Fatal("reconcile on the middle peer moved a held endpoint or overwrote the released source") + } +} + +// Default release turns AutoSync on. The periodic reconcile and a peer +// coming online call SyncAllGames, not the explicit sync route. That is +// enough to move bytes. This is the witness that "release does not call +// SyncGame" is not "nothing syncs until asked". +func TestProvisioningHold_DefaultReleaseLetsReconcileSync(t *testing.T) { + source := testutil.NewTestDaemon(t, "Reconcile-Source") + bridge := testutil.NewTestDaemon(t, "Reconcile-Bridge") + source.PairWith(bridge) + const id = "reconcile-game" + src := sideDir(t, source, "recon") + br := sideDir(t, bridge, "recon") + writeHoldFile(t, src, "slot.sav", "RECONCILE-BYTES") + createHeld(t, source, id, "Reconcile Game", src) + createHeld(t, bridge, id, "Reconcile Game", br) + source.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, nil) + bridge.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", nil, nil) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + source.Daemon.P2P.PingPairedPeers(ctx) + source.Daemon.P2P.SyncAllGames(ctx) + if !testutil.WaitFor(20*time.Second, func() bool { + return readHoldFile(t, br, "slot.sav") == "RECONCILE-BYTES" + }) { + t.Fatal("default release plus reconcile did not sync; the witness is wrong or the path changed") + } +} + +// Staged release clears the hold and leaves AutoSync off. Reconcile, a +// restart, and a file write must not move bytes. An explicit sync may, and +// a peer that is still held must refuse. Both ends of the star are tried as +// the writer. +func TestProvisioningHold_StagedReleaseSkipsReconcile(t *testing.T) { + for _, writer := range []string{"leafA", "leafB"} { + t.Run(writer, func(t *testing.T) { + leafA := testutil.NewTestDaemon(t, "Staged-LeafA") + hub := testutil.NewTestDaemon(t, "Staged-Hub") + leafB := testutil.NewTestDaemon(t, "Staged-LeafB") + leafA.PairWith(hub) + leafB.PairWith(hub) + const id = "staged-game" + bz := sideDir(t, leafA, "staged") + br := sideDir(t, hub, "staged") + dk := sideDir(t, leafB, "staged") + writeHoldFile(t, bz, "progress.sav", "LEAF-A-ONLY") + writeHoldFile(t, dk, "progress.sav", "LEAF-B-ONLY") + createHeld(t, leafA, id, "Staged Game", bz) + createHeld(t, hub, id, "Staged Game", br) + createHeld(t, leafB, id, "Staged Game", dk) + + progressed := leafA + progressedDir, quietDir, quietBytes := bz, dk, "LEAF-B-ONLY" + if writer == "leafB" { + progressed = leafB + progressedDir, quietDir, quietBytes = dk, bz, "LEAF-A-ONLY" + } + var rel struct { + Released bool `json:"released"` + AutoSync bool `json:"autoSync"` + } + progressed.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", map[string]any{"autoSync": false}, &rel) + hub.API(http.MethodPost, "/api/games/"+id+"/release-provisioning", map[string]any{"autoSync": false}, &rel) + if rel.AutoSync { + t.Fatal("staged release turned AutoSync on") + } + writeHoldFile(t, progressedDir, "later.sav", "AFTER-RELEASE") + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + for _, node := range []*testutil.TestDaemon{leafA, hub, leafB} { + node.Daemon.P2P.PingPairedPeers(ctx) + node.Daemon.P2P.SyncAllGames(ctx) + } + if readHoldFile(t, br, "progress.sav") != "" || readHoldFile(t, quietDir, "progress.sav") != quietBytes { + t.Fatalf("staged reconcile moved bytes: hub=%q quiet=%q", readHoldFile(t, br, "progress.sav"), readHoldFile(t, quietDir, "progress.sav")) + } + progressed.Restart() + hub.Restart() + progressed.Daemon.P2P.PingPairedPeers(ctx) + progressed.Daemon.P2P.SyncAllGames(ctx) + if readHoldFile(t, br, "progress.sav") != "" { + t.Fatal("restart after staged release let reconcile copy the save") + } + if !testutil.WaitFor(45*time.Second, func() bool { + progressed.API(http.MethodPost, "/api/games/"+id+"/sync", nil, nil) + return readHoldFile(t, br, "progress.sav") == map[string]string{"leafA": "LEAF-A-ONLY", "leafB": "LEAF-B-ONLY"}[writer] + }) { + t.Fatalf("explicit sync after staged release did not converge the released pair: hub=%q", readHoldFile(t, br, "progress.sav")) + } + if readHoldFile(t, br, "later.sav") != "AFTER-RELEASE" { + t.Fatal("explicit sync omitted later save progress") + } + for _, rel := range []string{"progress.sav", "later.sav"} { + want, got := fileHash(progressedDir, rel), fileHash(br, rel) + if want == "" || want != got { + t.Fatalf("sync hash differs for %q: source=%q hub=%q", rel, want, got) + } + } + if readHoldFile(t, quietDir, "progress.sav") != quietBytes || readHoldFile(t, quietDir, "later.sav") != "" { + t.Fatal("explicit sync moved the peer that is still held") + } + }) + } +} + +// A staged release blocks local auto-sync attempts but NOT an unheld +// peer's incoming request. This endpoint is not peer-specific authorization. +func TestProvisioningHold_StagedReleaseAllowsPeerInitiatedSync(t *testing.T) { + source := testutil.NewTestDaemon(t, "SyncRequester") + receiver := testutil.NewTestDaemon(t, "StagedReceiver") + source.WriteSave("slot.sav", "PEER-INITIATED") + gameID := source.TrackGame("Peer Initiated Staged Game") + recvDir := sideDir(t, receiver, "incoming") + createHeld(t, receiver, gameID, "Peer Initiated Staged Game", recvDir) + source.PairWith(receiver) + var state struct { + Released bool `json:"released"` + AutoSync bool `json:"autoSync"` + } + receiver.API(http.MethodPost, "/api/games/"+gameID+"/release-provisioning", + map[string]any{"autoSync": false}, &state) + if !state.Released || state.AutoSync { + t.Fatalf("staged release state = %+v", state) + } + if !testutil.WaitFor(45*time.Second, func() bool { + source.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + return readHoldFile(t, recvDir, "slot.sav") == "PEER-INITIATED" + }) { + t.Fatal("released receiver did not accept peer-initiated sync") + } + game, err := receiver.Daemon.Store.GetGame(gameID) + if err != nil || game.AutoSync { + t.Fatalf("peer request changed local AutoSync: %+v, %v", game, err) + } +} + +func TestProvisioningHold_PatchDoesNotUnhold(t *testing.T) { + source := testutil.NewTestDaemon(t, "Patch-Source") + bridge := testutil.NewTestDaemon(t, "Patch-Bridge") + source.PairWith(bridge) + const id = "patch-game" + src := sideDir(t, source, "patch") + br := sideDir(t, bridge, "patch") + writeHoldFile(t, src, "slot.sav", "PATCH-BYTES") + createHeld(t, source, id, "Patch Game", src) + createHeld(t, bridge, id, "Patch Game", br) + var patched struct { + Name string `json:"name"` + AutoSync bool `json:"autoSync"` + ProvisioningHold bool `json:"provisioningHold"` + ProvisioningHoldUnknown bool `json:"provisioningHoldUnknown"` + } + source.API(http.MethodPatch, "/api/games/"+id, map[string]any{ + "name": "Renamed", "savePath": src, "autoSync": true, + }, &patched) + if patched.Name != "Renamed" || patched.AutoSync || !patched.ProvisioningHold || patched.ProvisioningHoldUnknown { + t.Fatalf("patch changed the hold contract: %+v", patched) + } + held, err := source.Daemon.Store.ProvisioningHeld(id) + if err != nil || !held { + t.Fatal("PATCH cleared the hold") + } + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + source.Daemon.P2P.PingPairedPeers(ctx) + source.Daemon.P2P.SyncAllGames(ctx) + if readHoldFile(t, br, "slot.sav") != "" { + t.Fatal("PATCH autoSync true let reconcile copy a held save") + } +} + +// Path updates to a still-held game must reject symlink roots, as +// initial held creation and named-root admission already do. +func TestProvisioningHold_HeldPathUpdateRejectsSymlink(t *testing.T) { + node := testutil.NewTestDaemon(t, "HeldPathUpdate") + original := sideDir(t, node, "primary") + createHeld(t, node, "held-path-update", "Held Path Update", original) + target := sideDir(t, node, "target") + link := filepath.Join(filepath.Dir(node.SaveDir), "held-path-link") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlinks unsupported: %v", err) + } + var resp struct { + Error string `json:"error"` + } + status := node.APIStatus(http.MethodPatch, "/api/games/held-path-update", + map[string]any{"savePath": link}, &resp) + if status < 400 || status >= 500 { + t.Fatalf("held game accepted symlink update: HTTP %d %+v", status, resp) + } + game, err := node.Daemon.Store.GetGame("held-path-update") + if err != nil || game.SavePath != original || game.AutoSync { + t.Fatalf("rejected path update changed game: %+v, %v", game, err) + } + held, err := node.Daemon.Store.ProvisioningHeld(game.ID) + if err != nil || !held { + t.Fatalf("rejected path update removed hold: %v, %v", held, err) + } +} + +func TestProvisioningHold_OrdinaryTrackUnchanged(t *testing.T) { + node := testutil.NewTestDaemon(t, "Ordinary") + var got holdGame + node.API(http.MethodPost, "/api/games", map[string]string{"name": "Ordinary Game", "savePath": node.SaveDir}, &got) + if !got.AutoSync || got.ProvisioningHold { + t.Fatalf("ordinary track = %+v; omitting the hold must keep today's behaviour", got) + } +} diff --git a/internal/api/routes.go b/internal/api/routes.go index 65a28d0c..b3e9e1e2 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -29,6 +29,7 @@ func (s *Server) routes(r chi.Router) { r.Get("/api/games", s.handleListGames) r.Post("/api/games", s.handleTrackGame) + r.Post("/api/games/{gameId}/release-provisioning", s.handleReleaseProvisioning) r.Get("/api/suggest-name", s.handleSuggestName) r.Post("/api/games/untrack-bulk", s.handleBulkUntrack) @@ -339,9 +340,16 @@ func (s *Server) handlePruneSnapshots(w http.ResponseWriter, r *http.Request) { func (s *Server) handleTrackGame(w http.ResponseWriter, r *http.Request) { var body struct { - Name string `json:"name"` - SavePath string `json:"savePath"` - AppID string `json:"appId"` + ID string `json:"id"` + Name string `json:"name"` + SavePath string `json:"savePath"` + AppID string `json:"appId"` + ProvisioningHold bool `json:"provisioningHold"` + // AutoSync is only consulted when the caller sets it. Omitted keeps + // today's behaviour: track, then sync. false is a request to hold the + // game, because storing AutoSync off after the fact does not stop a + // peer that already knows the id from pulling the save. + AutoSync *bool `json:"autoSync"` } if err := readJSON(r, &body); err != nil { writeError(w, http.StatusBadRequest, err.Error()) @@ -351,8 +359,20 @@ func (s *Server) handleTrackGame(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "name and savePath are required") return } + hold := body.ProvisioningHold || (body.AutoSync != nil && !*body.AutoSync) + if body.ID != "" && !hold { + writeError(w, http.StatusBadRequest, "an explicit game id is only accepted when the game is created held") + return + } + if body.ID != "" && !store.ValidExplicitGameID(body.ID) { + writeError(w, http.StatusBadRequest, "game id must be a lowercase slug") + return + } - game, err := s.Daemon.TrackGame(store.Game{Name: body.Name, SavePath: body.SavePath, AppID: body.AppID}) + game, err := s.Daemon.TrackGame(store.Game{ + ID: body.ID, Name: body.Name, SavePath: body.SavePath, AppID: body.AppID, + ProvisioningHold: hold, + }) if err != nil { // Duplicates (id or path) are conflicts; anything else the daemon // rejects is bad input. @@ -379,11 +399,22 @@ func (s *Server) handleUpdateGame(w http.ResponseWriter, r *http.Request) { oldAutoSync := game.AutoSync oldIgnore := game.SyncIgnore oldAppID := game.AppID + heldBefore, holdErr := s.Daemon.StoreProvisioningHeld(gameID) + if holdErr != nil { + writeError(w, http.StatusServiceUnavailable, "could not read whether this game is still being configured") + return + } if err := readJSON(r, &game); err != nil { writeError(w, http.StatusBadRequest, err.Error()) return } game.ID = gameID // id is not client-mutable + // A hold is not a column on the game. A PATCH that sends autoSync, including + // a form that round-trips the whole object, must not clear it and must not + // turn the column on while the hold remains. Release is the only unblock. + if heldBefore { + game.AutoSync = oldAutoSync + } // A changed save path is validated exactly as a fresh track is. This // decoded straight into the stored game and wrote it back, so a path that @@ -401,6 +432,12 @@ func (s *Server) handleUpdateGame(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, err.Error()) return } + if heldBefore { + if err := daemon.RefuseSymlinkSave(abs); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + } game.SavePath = abs } // Cover art: a user-set custom URL is always kept. An empty cover, or @@ -455,10 +492,15 @@ func (s *Server) handleUpdateGame(w http.ResponseWriter, r *http.Request) { return } - // Re-watch if the save location or autoSync flag changed. + // Re-watch if the save location or autoSync flag changed. A hold still + // wins: turning autoSync on in the row must not start a watch that + // would sync a game that has not been released. if game.SavePath != oldSavePath || game.AutoSync != oldAutoSync { s.Daemon.Watcher.Unwatch(gameID) - if game.AutoSync { + held, holdErr := s.Daemon.StoreProvisioningHeld(gameID) + if holdErr != nil { + s.Daemon.Log.Log("warn", "not watching "+gameID+": provisioning hold could not be read: "+holdErr.Error()) + } else if game.AutoSync && !held { if err := s.Daemon.Watcher.Watch(gameID, game.SavePath); err != nil { s.Daemon.Log.Log("warn", "re-watch failed: "+err.Error()) } diff --git a/internal/api/routes_offered_games.go b/internal/api/routes_offered_games.go index a950d486..090c03a5 100644 --- a/internal/api/routes_offered_games.go +++ b/internal/api/routes_offered_games.go @@ -42,7 +42,9 @@ func (s *Server) handlePlaceOfferedGame(w http.ResponseWriter, r *http.Request) gameID := chi.URLParam(r, "gameId") var body struct { - Path string `json:"path"` + Path string `json:"path"` + ProvisioningHold bool `json:"provisioningHold"` + AutoSync *bool `json:"autoSync"` } if err := readJSON(r, &body); err != nil { writeError(w, http.StatusBadRequest, err.Error()) @@ -58,6 +60,22 @@ func (s *Server) handlePlaceOfferedGame(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusInternalServerError, err.Error()) return } + hold := body.ProvisioningHold || (body.AutoSync != nil && !*body.AutoSync) + if len(offers) == 0 && hold { + // The first place committed and the response was lost. Repeating it + // must not 404 into a second attempt that invents another game. + if _, err := s.Daemon.Store.GetGame(gameID); err == nil { + game, err := s.Daemon.TrackGame(store.Game{ + ID: gameID, Name: gameID, SavePath: body.Path, ProvisioningHold: true, + }) + if err != nil { + writeError(w, http.StatusConflict, err.Error()) + return + } + writeJSON(w, http.StatusOK, s.gamePayload(game)) + return + } + } if len(offers) == 0 { // Either it was placed or declined already, or the peer that offered // it has been unpaired. Not an error worth alarming anyone about. @@ -66,12 +84,16 @@ func (s *Server) handlePlaceOfferedGame(w http.ResponseWriter, r *http.Request) } offer := offers[0] + // autoSync: false is the request from issue #40. It is stored as a + // provisioning hold, not as the ordinary AutoSync flag: that flag does + // not stop a peer from pulling the save. Omitted keeps today's placement. game, err := s.Daemon.TrackGame(store.Game{ - ID: offer.GameID, - Name: offer.Name, - SavePath: body.Path, - AppID: offer.AppID, - CoverURL: offer.CoverURL, + ID: offer.GameID, + Name: offer.Name, + SavePath: body.Path, + AppID: offer.AppID, + CoverURL: offer.CoverURL, + ProvisioningHold: hold, }) if err != nil { // Path validation lives in TrackGame and its messages are written for diff --git a/internal/api/routes_peers.go b/internal/api/routes_peers.go index 4eb9b594..0b443e6f 100644 --- a/internal/api/routes_peers.go +++ b/internal/api/routes_peers.go @@ -252,6 +252,8 @@ func syncReason(err error) string { return "paused" case errors.Is(err, syncengine.ErrHeld): return "held" + case errors.Is(err, syncengine.ErrProvisioning): + return "provisioning" case errors.Is(err, p2p.ErrNoPeersOnline): return "offline" default: @@ -269,6 +271,15 @@ func (s *Server) handleSyncAll(w http.ResponseWriter, r *http.Request) { } results := map[string]any{} for _, g := range games { + held, holdErr := s.Daemon.StoreProvisioningHeld(g.ID) + if holdErr != nil { + results[g.ID] = map[string]string{"status": "error", "reason": "provisioning-unreadable", "error": holdErr.Error()} + continue + } + if held { + results[g.ID] = map[string]string{"status": "skipped", "reason": "provisioning"} + continue + } if !g.AutoSync { results[g.ID] = map[string]string{"status": "skipped", "reason": "autoSync disabled"} continue diff --git a/internal/api/routes_provisioning.go b/internal/api/routes_provisioning.go new file mode 100644 index 00000000..f71868e9 --- /dev/null +++ b/internal/api/routes_provisioning.go @@ -0,0 +1,57 @@ +package api + +import ( + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/opensave/opensave/internal/store" +) + +// handleReleaseProvisioning ends the hold on one game. It does not sync. +// +// An empty body turns AutoSync on and starts watching. Reconcile and a peer +// coming online will then sync this game with every online peer that is not +// still held. {"autoSync": false} clears the hold and leaves AutoSync off, so +// those paths do not sync. The caller syncs explicitly, then turns AutoSync +// on when that result is the one it wants kept. A repeat after the hold is +// gone does not change AutoSync. Staged release blocks this device +// from initiating automatic sync, not unheld peers from requesting files. +func (s *Server) handleReleaseProvisioning(w http.ResponseWriter, r *http.Request) { + gameID := chi.URLParam(r, "gameId") + var body struct { + AutoSync *bool `json:"autoSync"` + } + if r.ContentLength != 0 { + if err := readJSON(r, &body); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + } + enableAutoSync := true + if body.AutoSync != nil { + enableAutoSync = *body.AutoSync + } + released, err := s.Daemon.ReleaseProvisioningMode(gameID, enableAutoSync) + if err != nil { + status := http.StatusBadRequest + if errors.Is(err, store.ErrNotFound) { + status = http.StatusNotFound + } + writeError(w, status, err.Error()) + return + } + game, gerr := s.Daemon.Store.GetGame(gameID) + auto := enableAutoSync + if gerr == nil { + auto = game.AutoSync + } + s.BroadcastGamesUpdate() + writeJSON(w, http.StatusOK, map[string]any{ + "id": gameID, + "released": released, + "alreadyReleased": !released, + "autoSync": auto, + }) +} diff --git a/internal/api/routes_roots.go b/internal/api/routes_roots.go index 77915bf3..e3afa21d 100644 --- a/internal/api/routes_roots.go +++ b/internal/api/routes_roots.go @@ -6,6 +6,8 @@ import ( "strings" "github.com/go-chi/chi/v5" + + "github.com/opensave/opensave/internal/daemon" ) // A game's extra save locations: the folders beyond its main one that belong @@ -68,6 +70,17 @@ func (s *Server) handleAddGameRoot(w http.ResponseWriter, r *http.Request) { return } path = abs + held, holdErr := s.Daemon.StoreProvisioningHeld(gameID) + if holdErr != nil { + writeError(w, http.StatusServiceUnavailable, "could not read whether this game is still being configured") + return + } + if held { + if err := daemon.RefuseSymlinkSave(path); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + } } // Rejections here are the overlap and naming rules, which are the user's // to hear about rather than a server fault: "that folder is already diff --git a/internal/api/server.go b/internal/api/server.go index 66a32562..d83b6945 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -514,6 +514,20 @@ func (s *Server) gamesPayload() map[string]any { return out } +// provisioningHoldState reports the hold and whether that read failed. +// A failed read is not an ordinary false: held is true and unknown is true, +// so a client cannot treat the game as ready. +func (s *Server) provisioningHoldState(id string) (held, unknown bool) { + if s == nil || s.Daemon == nil { + return true, true + } + held, err := s.Daemon.StoreProvisioningHeld(id) + if err != nil { + return true, true + } + return held, false +} + func (s *Server) gamePayload(g store.Game) map[string]any { branchNames, _ := s.Daemon.Store.ListBranches(g.ID) branches := map[string]any{} @@ -545,17 +559,20 @@ func (s *Server) gamePayload(g store.Game) map[string]any { if since := s.Daemon.PlayingSince(g.ID); !since.IsZero() { playingSince = since.UTC().Format(time.RFC3339) } + hold, holdUnknown := s.provisioningHoldState(g.ID) return map[string]any{ - "id": g.ID, - "name": g.Name, - "savePath": g.SavePath, - "activeBranch": g.ActiveBranch, - "autoSync": g.AutoSync, - "maxSnapshots": g.MaxSnapshots, - "maxManualSnapshots": g.MaxManualSnapshots, - "appId": g.AppID, - "exePath": g.ExePath, - "coverUrl": g.CoverURL, + "id": g.ID, + "name": g.Name, + "savePath": g.SavePath, + "activeBranch": g.ActiveBranch, + "autoSync": g.AutoSync, + "provisioningHold": hold, + "provisioningHoldUnknown": holdUnknown, + "maxSnapshots": g.MaxSnapshots, + "maxManualSnapshots": g.MaxManualSnapshots, + "appId": g.AppID, + "exePath": g.ExePath, + "coverUrl": g.CoverURL, // Whether the art cached for this game is explicit, so the client can // blur it until someone asks to see it. An cannot read a // response header, so it travels with the game rather than the image. diff --git a/internal/cliapp/cliapp.go b/internal/cliapp/cliapp.go index dacaa2ac..9e800243 100644 --- a/internal/cliapp/cliapp.go +++ b/internal/cliapp/cliapp.go @@ -571,6 +571,20 @@ func loadScanResults(homeDir string) []scanChoice { } func cmdAdd(d *daemon.Daemon, args []string) int { + hold, args := holdFlag(args) + explicitID, args, err := idFlag(args) + if err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + return 1 + } + if explicitID != "" && !hold { + fmt.Fprintln(os.Stderr, "error: --id is only accepted with --hold") + return 1 + } + if explicitID != "" && !store.ValidExplicitGameID(explicitID) { + fmt.Fprintln(os.Stderr, "error: --id must be a lowercase slug") + return 1 + } // A single numeric argument means "the nth thing the last scan showed". // Unambiguous against the two-argument form, so a game genuinely called // "3" is still trackable by naming its path. @@ -587,7 +601,7 @@ func cmdAdd(d *daemon.Daemon, args []string) int { fmt.Fprintln(os.Stderr, "error: nothing in that path says which game it is — give a name: opensave add ") return 1 } - return trackGame(d, name, args[0]) + return trackGame(d, name, args[0], hold, explicitID) } choices := loadScanResults(d.Paths.HomeDir) if len(choices) == 0 { @@ -599,31 +613,37 @@ func cmdAdd(d *daemon.Daemon, args []string) int { return 1 } pick := choices[n-1] - return trackGame(d, pick.Name, pick.SavePath) + return trackGame(d, pick.Name, pick.SavePath, hold, explicitID) } if len(args) < 2 { - fmt.Fprintln(os.Stderr, "usage: opensave add \n opensave add (named from the path)\n opensave add (from the last `opensave scan`)") + fmt.Fprintln(os.Stderr, "usage: opensave add \n opensave add --hold [--id ] \n opensave add (named from the path)\n opensave add (from the last `opensave scan`)") return 1 } - return trackGame(d, args[0], args[1]) + return trackGame(d, args[0], args[1], hold, explicitID) } // trackGame is the shared tail of both `add` forms, so picking a scan result // and typing a path by hand cannot drift apart in what they report. -func trackGame(d *daemon.Daemon, name, savePath string) int { +func trackGame(d *daemon.Daemon, name, savePath string, hold bool, explicitID string) int { abs, err := filepath.Abs(savePath) if err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) return 1 } - game, err := d.TrackGame(store.Game{Name: name, SavePath: abs}) + game, err := d.TrackGame(store.Game{ + ID: explicitID, Name: name, SavePath: abs, ProvisioningHold: hold, + }) if err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) return 1 } - success("Now tracking %s", bold(game.Name)) + if hold { + success("Now tracking %s without syncing — release it when it is configured", bold(game.Name)) + } else { + success("Now tracking %s", bold(game.Name)) + } note("id: " + game.ID) note("path: " + game.SavePath) return 0 diff --git a/internal/cliapp/client.go b/internal/cliapp/client.go index 70193426..90af34d6 100644 --- a/internal/cliapp/client.go +++ b/internal/cliapp/client.go @@ -138,6 +138,42 @@ func daemonRunning() bool { // Every command can emit JSON so the CLI is scriptable — the point of a // headless client is being driven by something other than a human. +// holdFlag reports whether --hold was passed, and returns the remaining args. +func holdFlag(args []string) (bool, []string) { + return stripFlag(args, "--hold") +} + +// idFlag reports a --id and returns the remaining args. +func idFlag(args []string) (string, []string, error) { + out := make([]string, 0, len(args)) + id := "" + for i := 0; i < len(args); i++ { + if args[i] != "--id" { + out = append(out, args[i]) + continue + } + if i+1 >= len(args) { + return "", nil, fmt.Errorf("--id requires a game id") + } + id = args[i+1] + i++ + } + return id, out, nil +} + +func stripFlag(args []string, flag string) (bool, []string) { + out := make([]string, 0, len(args)) + found := false + for _, a := range args { + if a == flag { + found = true + continue + } + out = append(out, a) + } + return found, out +} + // jsonFlag reports whether --json was passed, and returns the remaining args. func jsonFlag(args []string) (bool, []string) { out := args[:0:0] diff --git a/internal/cliapp/commands_games.go b/internal/cliapp/commands_games.go index 4e3f53c4..7e571729 100644 --- a/internal/cliapp/commands_games.go +++ b/internal/cliapp/commands_games.go @@ -20,6 +20,9 @@ import ( // cmdGame edits one game's settings. func cmdGame(d *daemon.Daemon, args []string) int { asJSON, args := jsonFlag(args) + if len(args) >= 2 && args[1] == "release" { + return releaseProvisioning(d, args, asJSON) + } if len(args) < 2 || args[1] != "set" { fmt.Fprintln(os.Stderr, gameUsage) return 1 @@ -49,6 +52,13 @@ func cmdGame(d *daemon.Daemon, args []string) int { case "cover-url": game.CoverURL = value case "auto-sync": + held, holdErr := d.StoreProvisioningHeld(game.ID) + if holdErr != nil { + return fail(asJSON, fmt.Errorf("could not read whether this game is still being configured: %w", holdErr)) + } + if held { + return fail(asJSON, fmt.Errorf("%s is still being configured; release it before changing auto-sync", game.ID)) + } game.AutoSync = isTruthy(value) case "max-snapshots": n, err := strconv.Atoi(value) @@ -80,7 +90,10 @@ func cmdGame(d *daemon.Daemon, args []string) int { // Re-watch so a path or auto-sync change takes effect immediately rather // than at the next restart. d.Watcher.Unwatch(game.ID) - if game.AutoSync { + held, holdErr := d.StoreProvisioningHeld(game.ID) + if holdErr != nil { + d.Log.Log("warn", "not watching "+game.ID+": provisioning hold could not be read: "+holdErr.Error()) + } else if game.AutoSync && !held { if err := d.Watcher.Watch(game.ID, game.SavePath); err != nil { d.Log.Log("warn", "re-watch after config change failed: "+err.Error()) } @@ -93,7 +106,73 @@ func cmdGame(d *daemon.Daemon, args []string) int { return 0 } +func gameIDFrom(args []string) string { + if len(args) == 0 { + return "" + } + return args[0] +} + +func releaseProvisioning(d *daemon.Daemon, args []string, asJSON bool) int { + noAuto, rest := stripFlag(args[2:], "--no-autosync") + if len(args) < 2 || len(rest) != 0 { + fmt.Fprintln(os.Stderr, gameUsage) + return 1 + } + gameID := gameIDFrom(args) + if gameID == "" { + fmt.Fprintln(os.Stderr, gameUsage) + return 1 + } + body := map[string]any{} + if noAuto { + body["autoSync"] = false + } + // Prefer the running daemon so a default release starts the watch there. + // --no-autosync must not: reconcile would otherwise sync the game. + if daemonRunning() { + raw, err := daemonRequest("POST", "/api/games/"+gameID+"/release-provisioning", body) + if err != nil { + return fail(asJSON, err) + } + if asJSON { + return emitRawJSON(raw) + } + if noAuto { + success("Released %s without auto-sync. Sync it explicitly, then turn auto-sync on.", bold(gameID)) + } else { + success("Released %s. Reconcile can sync it from here on.", bold(gameID)) + } + return 0 + } + released, err := d.ReleaseProvisioningMode(gameID, !noAuto) + if err != nil { + return fail(asJSON, err) + } + if asJSON { + // An idempotent repeat changes nothing. Return stored AutoSync, + // not the mode requested by this invocation. + game, err := d.Store.GetGame(gameID) + if err != nil { + return fail(asJSON, err) + } + return emitJSON(map[string]any{"id": gameID, "released": released, "alreadyReleased": !released, "autoSync": game.AutoSync}) + } + if released && noAuto { + success("Released %s without auto-sync. Sync it explicitly, then turn auto-sync on.", bold(gameID)) + } else if released { + success("Released %s. Reconcile can sync it from here on.", bold(gameID)) + } else { + success("%s was already released.", bold(gameID)) + } + return 0 +} + const gameUsage = `usage: opensave game set + opensave game release [--no-autosync] + clear a provisioning hold. Without --no-autosync, + reconcile may sync it. With --no-autosync, it stays + quiet until an explicit sync. name Display name (also how peers match this game) path Move tracking to a different save location diff --git a/internal/cliapp/commands_offers.go b/internal/cliapp/commands_offers.go index baa4805a..aa459198 100644 --- a/internal/cliapp/commands_offers.go +++ b/internal/cliapp/commands_offers.go @@ -24,10 +24,13 @@ type offeredGame struct { const offersUsage = `usage: opensave offers games another device syncs, waiting for a folder here opensave offers place track it here, in that folder + opensave offers place --hold + track it held, so it does not sync until released opensave offers decline don't — it is not offered again` func cmdOffers(args []string) int { asJSON, args := jsonFlag(args) + hold, args := holdFlag(args) if len(args) == 0 || args[0] == "list" { return listOffers(asJSON) } @@ -41,7 +44,11 @@ func cmdOffers(args []string) int { if err != nil { return fail(asJSON, err) } - raw, err := daemonRequest("POST", "/api/offered-games/"+url.PathEscape(args[1])+"/place", map[string]string{"path": folder}) + body := map[string]any{"path": folder} + if hold { + body["provisioningHold"] = true + } + raw, err := daemonRequest("POST", "/api/offered-games/"+url.PathEscape(args[1])+"/place", body) if err != nil { return fail(asJSON, err) } @@ -52,7 +59,11 @@ func cmdOffers(args []string) int { Name string `json:"name"` } _ = json.Unmarshal(raw, &game) - success("Now tracking %s at %s — it syncs from here on.", bold(orNone(game.Name)), folder) + if hold { + success("Now tracking %s at %s without syncing — release it when it is configured.", bold(orNone(game.Name)), folder) + } else { + success("Now tracking %s at %s — it syncs from here on.", bold(orNone(game.Name)), folder) + } return 0 case "decline": if len(args) < 2 { diff --git a/internal/daemon/cloud_retry.go b/internal/daemon/cloud_retry.go index ea8d06e8..b22e0f0e 100644 --- a/internal/daemon/cloud_retry.go +++ b/internal/daemon/cloud_retry.go @@ -2,6 +2,7 @@ package daemon import ( "fmt" + "strings" "time" "github.com/opensave/opensave/internal/cloud" @@ -60,6 +61,9 @@ func (d *Daemon) retryFailedUploads(inCloud []cloud.CloudFile) { } d.Log.Log("info", "cloud: sending again "+what+" that did not go up earlier") for _, p := range send { + if id, _, ok := strings.Cut(p.RemoteName, "__"); ok && d.provisioningBlocks(id) { + continue + } d.uploads.Add() d.runCloudUpload(p.ZipPath, p.RemoteName, d.Log) } diff --git a/internal/daemon/cloudsync.go b/internal/daemon/cloudsync.go index 6b4dbbf5..d41e6357 100644 --- a/internal/daemon/cloudsync.go +++ b/internal/daemon/cloudsync.go @@ -534,7 +534,7 @@ func (d *Daemon) CheckCloud() { if !follows && d.neverHeldFiles(game) { follows, state = true, saveUnchanged } - if follows && state == saveUnchanged && settings.CloudAutoPull && game.AutoSync { + if follows && state == saveUnchanged && settings.CloudAutoPull && game.AutoSync && !d.provisioningBlocks(game.ID) { err := d.pullFromCloud(game, cand.file, cand.head.DeviceName) if err == nil { if d.OnCloudPulled != nil { diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 306ce6ff..fc79f5f1 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -197,6 +197,12 @@ func New(opts Options) (*Daemon, error) { // Counted here, on the caller's goroutine, and only then moved to the // background, so that Stop, once it has seen the snapshot finish, // also sees its upload. + // A game still being configured must not publish its snapshot. The + // name is gameID__branch__snapID.zip (cloudSnapshotName). Checked + // before the pause queue: a resume must not send it either. + if id, _, ok := strings.Cut(remoteFileName, "__"); ok && d.provisioningBlocks(id) { + return + } if d.P2P.Pause.Paused() { d.holdUpload(zipPath, remoteFileName) return @@ -285,7 +291,7 @@ func (d *Daemon) Start() error { _ = d.Store.UpdateGame(game) } } - if !game.AutoSync { + if !game.AutoSync || d.provisioningBlocks(game.ID) { continue } if err := d.watchGame(game.ID, game.SavePath); err != nil { @@ -617,7 +623,7 @@ func (d *Daemon) ResyncWatchers() (started, stopped int) { names := make(map[string]string, len(games)) for _, game := range games { names[game.ID] = game.Name - if game.AutoSync { + if game.AutoSync && !d.provisioningBlocks(game.ID) { want[game.ID] = game.SavePath } } @@ -682,6 +688,16 @@ func isDuplicateGameID(err error) bool { func (d *Daemon) TrackGame(game store.Game) (store.Game, error) { abs, err := d.ValidateSavePath(game.SavePath) if err != nil { + // A retry of a held create names a folder this game already tracks. + // That is the same admission, not a second game. Checked here because + // the duplicate-path refusal otherwise wins before the hold path runs. + if game.ProvisioningHold && game.ID != "" { + if existing, findErr := d.Store.GetGame(game.ID); findErr == nil { + if reused, reuseErr := d.reuseHeldGame(existing, game.SavePath); reuseErr == nil { + return reused, nil + } + } + } return store.Game{}, err } game.SavePath = abs @@ -716,6 +732,13 @@ func (d *Daemon) TrackGame(game store.Game) (store.Game, error) { game.ID = id } + // A game still being configured is recorded held, and nothing is said to + // a peer until that hold is released. NotifyRetrack and the sync below + // are how a new game becomes visible; both have to stay off here. + if game.ProvisioningHold { + return d.trackProvisioningHold(game) + } + // Explicit (re)tracking overrides any earlier untrack: clear the local // tombstone, and tell peers to clear theirs too so a peer that mirrored // an earlier untrack will accept this game again (otherwise its @@ -823,6 +846,159 @@ func (d *Daemon) TrackGame(game store.Game) (store.Game, error) { return created, nil } +// trackProvisioningHold records a game that must not sync until released. +// The hold is committed with the row. No retrack, no watch, no peer sync. +func (d *Daemon) trackProvisioningHold(game store.Game) (store.Game, error) { + if err := refuseSymlinkSave(game.SavePath); err != nil { + return store.Game{}, err + } + if existing, err := d.Store.FindGameBySavePath(game.SavePath); err == nil { + if game.ID != "" && existing.ID != game.ID { + return store.Game{}, fmt.Errorf("%q already tracks this folder", existing.Name) + } + return d.reuseHeldGame(existing, game.SavePath) + } + if game.ID != "" { + if existing, err := d.Store.GetGame(game.ID); err == nil { + return d.reuseHeldGame(existing, game.SavePath) + } + } + game.AutoSync = false + if game.ActiveBranch == "" { + game.ActiveBranch = "main" + } + if err := d.Store.CreateHeldGame(game); err != nil { + if isDuplicateGameID(err) { + if existing, getErr := d.Store.GetGame(game.ID); getErr == nil { + return d.reuseHeldGame(existing, game.SavePath) + } + } + return store.Game{}, err + } + d.setUpRegistryCapture(game) + d.initialSnapshots.Add() + go func() { + defer d.initialSnapshots.Done() + if _, err := d.Snapshots.Create(game.ID, "Initial snapshot", true); err != nil { + d.Log.Log("warn", fmt.Sprintf("initial snapshot for %q failed: %v", game.Name, err)) + } + // Deliberately no watch and no SyncGame. Watching would push the + // first change, and syncing would publish the game to paired peers. + d.Log.Log("info", fmt.Sprintf("tracking %q at %s without syncing; it is still being configured", game.Name, logging.Quote(game.SavePath))) + }() + created, err := d.Store.GetGame(game.ID) + if err != nil { + return store.Game{}, err + } + return created, nil +} + +// reuseHeldGame is a retry of a create that already committed. Same id and +// same folder returns the existing row. Anything else is a conflict: a live +// game must not be turned into a hold, and a held game must not be moved. +func (d *Daemon) reuseHeldGame(existing store.Game, abs string) (store.Game, error) { + held, err := d.Store.ProvisioningHeld(existing.ID) + if err != nil { + return store.Game{}, err + } + if !held { + return store.Game{}, fmt.Errorf("%q already exists", existing.Name) + } + if !sameSaveLocation(existing.SavePath, abs) { + return store.Game{}, fmt.Errorf("%q is already being configured at %q", existing.Name, existing.SavePath) + } + return existing, nil +} + +func sameSaveLocation(a, b string) bool { + if strings.EqualFold(filepath.Clean(a), filepath.Clean(b)) { + return true + } + ai, aerr := os.Stat(a) + bi, berr := os.Stat(b) + return aerr == nil && berr == nil && os.SameFile(ai, bi) +} + +// refuseSymlinkSave rejects a save location that is itself a symlink. +// Tracking the link would follow it and sync whatever it points at, including +// a folder outside the directory the caller named. +func refuseSymlinkSave(path string) error { + info, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("save path does not exist: %s", path) + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("refusing a symlink as a save location: %s", path) + } + return nil +} + +// StoreProvisioningHeld reports whether gameID, or an alias of it, is still +// being configured. A non-nil error means the hold could not be read; that +// is not the same as false. +func (d *Daemon) StoreProvisioningHeld(gameID string) (bool, error) { + return d.provisioningHeld(gameID) +} + +// RefuseSymlinkSave rejects a save location that is itself a symlink. +func RefuseSymlinkSave(path string) error { + return refuseSymlinkSave(path) +} + +func (d *Daemon) provisioningHeld(gameID string) (bool, error) { + if d == nil || d.Store == nil { + if gameID == "" { + return false, nil + } + return false, fmt.Errorf("provisioning hold could not be read") + } + return d.Store.ProvisioningBlocks(gameID) +} + +// provisioningBlocks is true when the game is held or the hold cannot be +// read. Callers that would otherwise sync, watch, or upload must use this +// and not a bare false. +func (d *Daemon) provisioningBlocks(gameID string) bool { + held, err := d.provisioningHeld(gameID) + if err != nil { + if d.Log != nil { + d.Log.Log("warn", fmt.Sprintf("treating %s as not ready to sync: provisioning hold could not be read: %v", gameID, err)) + } + return true + } + return held +} + +// ReleaseProvisioning clears the hold, turns AutoSync on, and starts watching. +// It does not call SyncGame. Reconcile and a peer coming online will, because +// AutoSync is on. Use ReleaseProvisioningMode when that must not happen yet. +func (d *Daemon) ReleaseProvisioning(gameID string) (bool, error) { + return d.ReleaseProvisioningMode(gameID, true) +} + +// ReleaseProvisioningMode clears the hold. enableAutoSync false does not +// watch and does not turn AutoSync on, so reconcile, reconnect and a file +// change do not sync. An explicit sync still can, and a peer that is still +// held still refuses. A second call is a no-op and does not change AutoSync. +func (d *Daemon) ReleaseProvisioningMode(gameID string, enableAutoSync bool) (bool, error) { + game, err := d.Store.GetGame(gameID) + if err != nil { + return false, err + } + released, err := d.Store.ReleaseProvisioningMode(gameID, enableAutoSync) + if err != nil || !released { + return released, err + } + if enableAutoSync { + if err := d.watchGame(game.ID, game.SavePath); err != nil && + !errors.Is(err, watcher.ErrStopped) && !errors.Is(err, watcher.ErrSaveFolderMissing) { + d.Log.Log("warn", fmt.Sprintf("could not watch %q after release: %v", game.Name, err)) + } + } + d.Log.Log("info", fmt.Sprintf("released %q (autoSync=%t); it is not synced by this call", game.Name, enableAutoSync)) + return true, nil +} + // validateSavePath rejects save locations that can never be right: paths // that don't exist, whole-profile/system directories, drive roots, // OpenSave's own data directory, and paths another game already tracks. @@ -1045,6 +1221,8 @@ func (d *Daemon) UntrackGame(gameID string) error { if game, err := d.Store.GetGame(gameID); err == nil { name, savePath = game.Name, game.SavePath } + // Read before the delete: the hold row cascades away with the game. + held := d.provisioningBlocks(gameID) if err := d.Store.DeleteGame(gameID); err != nil { return err } @@ -1064,7 +1242,12 @@ func (d *Daemon) UntrackGame(gameID string) error { // that went missing while untracked as a deletion to propagate. _ = d.Store.ForgetGameSyncState(gameID) d.P2P.ClearPendingResync(gameID) - d.P2P.NotifyUntrack(gameID) + // A held game was never published. Telling peers to untrack it would + // remove a copy that the other device is still configuring, or one it + // has already released, because of an abort on this side. + if !held { + d.P2P.NotifyUntrack(gameID) + } return nil } @@ -1079,6 +1262,9 @@ func (d *Daemon) LinkGames(canonicalID, aliasID string) error { if canonicalID == "" || aliasID == "" || canonicalID == aliasID { return fmt.Errorf("invalid link %q -> %q", aliasID, canonicalID) } + if d.provisioningBlocks(canonicalID) || d.provisioningBlocks(aliasID) { + return fmt.Errorf("a game still being configured cannot be linked") + } if _, err := d.Store.GetGame(canonicalID); err != nil { return fmt.Errorf("canonical game %q: %w", canonicalID, err) } @@ -1197,6 +1383,10 @@ func (d *Daemon) UnlinkGame(aliasID string) error { // untrackFromPeer mirrors a peer's untrack: remove the game + tombstone it, // WITHOUT re-notifying (no loop). func (d *Daemon) untrackFromPeer(gameID string) { + if d.provisioningBlocks(gameID) { + d.Log.Log("info", fmt.Sprintf("ignored a peer untrack of %q while it is still being configured", gameID)) + return + } d.Watcher.Unwatch(gameID) // Remember where THIS device kept the game before the record goes. A // re-track on the peer used to bring the game back by auto-tracking from @@ -1289,7 +1479,7 @@ func (d *Daemon) watchGame(gameID, savePath string) error { // nobody is covering any more. func (d *Daemon) RewatchGame(gameID string) { game, err := d.Store.GetGame(gameID) - if err != nil || !game.AutoSync { + if err != nil || !game.AutoSync || d.provisioningBlocks(gameID) { return } if err := d.watchGame(game.ID, game.SavePath); err != nil { diff --git a/internal/p2p/engine.go b/internal/p2p/engine.go index b1777e2e..afa9d370 100644 --- a/internal/p2p/engine.go +++ b/internal/p2p/engine.go @@ -444,6 +444,16 @@ func (e *Engine) SyncGame(ctx context.Context, gameID string) (map[string]syncen if e.Pause.Paused() { return nil, syncengine.ErrPaused } + // Before pinging anyone. A held game must not be the reason a peer is + // contacted, and the refusal must not depend on a peer being online. + if err := e.refuseIfProvisioning(gameID); err != nil { + return nil, err + } + if resolved := e.localGameID(gameID); resolved != gameID { + if err := e.refuseIfProvisioning(resolved); err != nil { + return nil, err + } + } gameID = e.localGameID(gameID) e.PingPairedPeers(ctx) online := e.OnlinePeers() @@ -602,6 +612,17 @@ func (e *Engine) retryPendingResyncs(ctx context.Context) { if ctx.Err() != nil { return // shutting down; the failsafe picks these up next start } + held, holdErr := e.provisioningHeld(id) + if holdErr != nil { + e.Log("warn", fmt.Sprintf("not retrying %s: provisioning hold could not be read: %v", id, holdErr)) + continue + } + if held { + e.pendingMu.Lock() + delete(e.pendingResync, id) + e.pendingMu.Unlock() + continue + } e.Log("info", fmt.Sprintf("retrying interrupted sync for %s", id)) results, err := e.Sync.SyncGame(ctx, id, online) if err == nil { @@ -616,6 +637,43 @@ func (e *Engine) retryPendingResyncs(ctx context.Context) { } } +func (e *Engine) provisioningHeld(gameID string) (bool, error) { + if e == nil || e.Store == nil { + if gameID == "" { + return false, nil + } + return false, fmt.Errorf("provisioning hold could not be read") + } + return e.Store.ProvisioningBlocks(gameID) +} + +// provisioningServeRefusal is the HTTP answer for a peer asking for a game +// that is held, or whose hold cannot be read. refuse is false only when the +// game is confirmed not held. +func (e *Engine) provisioningServeRefusal(gameID string) (refuse bool, status int, msg string) { + held, err := e.provisioningHeld(gameID) + if err != nil { + return true, http.StatusServiceUnavailable, syncengine.ProvisioningUnreadableMessage + } + if held { + return true, http.StatusConflict, syncengine.ProvisioningMessage + } + return false, 0, "" +} + +// refuseIfProvisioning stops a sync when the game is held or the hold cannot +// be read. A lookup failure is not permission to sync. +func (e *Engine) refuseIfProvisioning(gameID string) error { + held, err := e.provisioningHeld(gameID) + if err != nil { + return fmt.Errorf("%w: %v", syncengine.ErrProvisioningUnreadable, err) + } + if held { + return syncengine.ErrProvisioning + } + return nil +} + // SyncAllGames syncs every tracked game (used when a peer comes online). func (e *Engine) SyncAllGames(ctx context.Context) { if e.Pause.Paused() { @@ -629,8 +687,13 @@ func (e *Engine) SyncAllGames(ctx context.Context) { if len(online) == 0 { return } + held, err := e.Store.ProvisioningHeldSet() + if err != nil { + e.Log("warn", "not syncing: provisioning holds could not be read: "+err.Error()) + return + } for _, g := range games { - if !g.AutoSync { + if _, skip := held[g.ID]; !g.AutoSync || skip { continue } results, err := e.Sync.SyncGame(ctx, g.ID, online) diff --git a/internal/p2p/provisioning_fault_test.go b/internal/p2p/provisioning_fault_test.go new file mode 100644 index 00000000..b9454a3c --- /dev/null +++ b/internal/p2p/provisioning_fault_test.go @@ -0,0 +1,40 @@ +package p2p + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/opensave/opensave/internal/store" + "github.com/opensave/opensave/internal/syncpause" +) + +// SyncAllGames used to ignore a failed hold-set read and sync every AutoSync +// game. With a peer online and the game row still readable, that is a transfer. +// Sync is nil here: reaching it panics, which is the failure this test wants +// if the early return is removed. +func TestSyncAllGamesStopsWhenHoldSetUnreadable(t *testing.T) { + s, err := store.Open(filepath.Join(t.TempDir(), "opensave.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { s.Close() }) + if err := s.EnsureDefaultSettings(t.TempDir(), t.TempDir()); err != nil { + t.Fatal(err) + } + if err := s.CreateGame(store.Game{ + ID: "live", Name: "Live", SavePath: t.TempDir(), ActiveBranch: "main", + AutoSync: true, MaxSnapshots: 5, + }); err != nil { + t.Fatal(err) + } + if err := s.UpsertPeer(store.Peer{ + ID: "peer", Name: "Peer", Address: "127.0.0.1", Port: 9, Status: "online", + }); err != nil { + t.Fatal(err) + } + s.SetProvisioningReadFault(errors.New("injected hold read failure")) + e := &Engine{Store: s, Pause: syncpause.New(), Log: func(string, string) {}} + e.SyncAllGames(context.Background()) +} diff --git a/internal/p2p/routes.go b/internal/p2p/routes.go index 67d11a5c..6378efd9 100644 --- a/internal/p2p/routes.go +++ b/internal/p2p/routes.go @@ -354,20 +354,34 @@ type PeerGame struct { // would put a device's whole library on the wire continuously to say // nothing that changes. This is asked for once, when a human opens the // picker. -func (e *Engine) PeerGameList() []PeerGame { +func (e *Engine) PeerGameList() ([]PeerGame, error) { games, err := e.Store.ListGames() if err != nil { - return []PeerGame{} + return nil, err + } + held, err := e.Store.ProvisioningHeldSet() + if err != nil { + return nil, err } out := make([]PeerGame, 0, len(games)) for _, g := range games { + // A game still being configured is not offered for linking. Linking + // would publish its id, and the peer would then ask for its saves. + if _, skip := held[g.ID]; skip { + continue + } out = append(out, PeerGame{ID: g.ID, Name: g.Name, SavePath: g.SavePath, AppID: g.AppID}) } - return out + return out, nil } func (e *Engine) handlePeerGameList(w http.ResponseWriter, r *http.Request) { - jsonOK(w, e.PeerGameList()) + list, err := e.PeerGameList() + if err != nil { + jsonError(w, http.StatusServiceUnavailable, syncengine.ProvisioningUnreadableMessage) + return + } + jsonOK(w, list) } // FetchPeerGames asks a paired peer what it is tracking, over whichever @@ -678,6 +692,10 @@ func (e *Engine) handleManifest(w http.ResponseWriter, r *http.Request) { jsonError(w, http.StatusNotFound, syncengine.HeldMessage) return } + if refuse, status, msg := e.provisioningServeRefusal(game.ID); refuse { + jsonError(w, status, msg) + return + } // Never describe a save a sync here is writing: part-way through, it is a // mixture no device holds, and the asker would judge it as a save that had // moved (syncengine/settle.go). Held still while it is read; a write in @@ -759,6 +777,10 @@ func (e *Engine) handleBlocks(w http.ResponseWriter, r *http.Request) { jsonError(w, http.StatusNotFound, "Game not found.") return } + if refuse, status, msg := e.provisioningServeRefusal(game.ID); refuse { + jsonError(w, status, msg) + return + } base, ok := e.resolveServeRoot(gameID, game, body.Root) if !ok { jsonError(w, http.StatusNotFound, "This device has no save location named "+strconv.Quote(body.Root)+" for that game.") @@ -803,6 +825,10 @@ func (e *Engine) handleDeleteFile(w http.ResponseWriter, r *http.Request) { jsonError(w, http.StatusNotFound, "Game not found.") return } + if refuse, status, msg := e.provisioningServeRefusal(game.ID); refuse { + jsonError(w, status, msg) + return + } base, ok := e.resolveServeRoot(gameID, game, body.Root) if !ok { jsonError(w, http.StatusNotFound, "This device has no save location named "+strconv.Quote(body.Root)+" for that game.") @@ -962,10 +988,14 @@ func (e *Engine) peerByAddress(ip string) (syncengine.Peer, bool) { // newer content for us to pull. func (e *Engine) handleSyncTrigger(w http.ResponseWriter, r *http.Request) { gameID := chi.URLParam(r, "gameId") + if refuse, status, msg := e.provisioningServeRefusal(gameID); refuse { + jsonError(w, status, msg) + return + } e.GoSync(func(ctx context.Context) { ctx, cancel := context.WithTimeout(ctx, 10*time.Minute) defer cancel() - if _, err := e.SyncGame(ctx, gameID); err != nil && !errors.Is(err, syncengine.ErrHeld) { + if _, err := e.SyncGame(ctx, gameID); err != nil && !errors.Is(err, syncengine.ErrHeld) && !errors.Is(err, syncengine.ErrProvisioning) { e.Log("warn", fmt.Sprintf("triggered sync for %s: %v", gameID, err)) } }) diff --git a/internal/p2p/syncengine/engine.go b/internal/p2p/syncengine/engine.go index e3b02b00..3f519129 100644 --- a/internal/p2p/syncengine/engine.go +++ b/internal/p2p/syncengine/engine.go @@ -70,7 +70,9 @@ type Result struct { // peer_missing: the peer does not track this game. peer_awaiting_folder: // the peer knows about it but is waiting for someone to choose a folder. // peer_holding: the peer's save was emptied and it is holding the game - // back until told whether that was meant (hold.go). peer_busy: the peer was + // back until told whether that was meant (hold.go). peer_provisioning: + // the peer is still configuring the game and must not be read as empty. + // peer_busy: the peer was // still writing the game for a sync of its own; this one runs again when it // has finished (settle.go). Status string `json:"status"` // in_sync | updated | updated_bidirectional | deletions_synced | triggered_peer_pull | conflict | peer_missing | peer_awaiting_folder | peer_holding | peer_busy @@ -228,6 +230,16 @@ func (e *Engine) SyncBusy(gameID string) bool { return e.activeSyncs[gameID] || e.pendingSyncs[gameID] || e.followUps[gameID] > 0 } +func (e *Engine) provisioningHeld(gameID string) (bool, error) { + if e == nil || e.Store == nil { + if gameID == "" { + return false, nil + } + return false, fmt.Errorf("provisioning hold could not be read") + } + return e.Store.ProvisioningBlocks(gameID) +} + func (e *Engine) SyncGame(ctx context.Context, gameID string, onlinePeers []Peer) (map[string]Result, error) { // Every sync this device starts comes through here — a watched change, a // peer coming online, the periodic reconcile, a sync asked for by hand or @@ -240,6 +252,15 @@ func (e *Engine) SyncGame(ctx context.Context, gameID string, onlinePeers []Peer if held, err := e.CheckHold(gameID, false); err == nil && held { return nil, ErrHeld } + // A game still being configured does not sync, whichever caller got + // here: a watch, a retry, a peer trigger, or a sync asked for by hand. + held, holdErr := e.provisioningHeld(gameID) + if holdErr != nil { + return nil, fmt.Errorf("%w: %v", ErrProvisioningUnreadable, holdErr) + } + if held { + return nil, ErrProvisioning + } e.mu.Lock() if e.activeSyncs[gameID] { e.pendingSyncs[gameID] = true @@ -335,7 +356,7 @@ func (e *Engine) SyncGame(ctx context.Context, gameID string, onlinePeers []Peer // its own changes instead of detecting the conflict and asking. switch res.Status { case "conflict", "error": - case "peer_missing", "peer_awaiting_folder", "peer_holding": + case "peer_missing", "peer_awaiting_folder", "peer_holding", "peer_provisioning": // The two devices talked and finished, which is what the // per-device stamp has always recorded. But nothing of THIS game // moved — the peer does not track it, or is still waiting to be @@ -430,6 +451,12 @@ func (e *Engine) SyncWithPeer(ctx context.Context, gameID string, peer Peer) (Re if isHeld(err) { return Result{Status: "peer_holding", PeerID: peer.ID, PeerName: peer.Name}, nil } + // The peer has the game and is still configuring it. Nothing is + // taken from it and nothing is sent: an empty answer would be read + // as a deleted save. + if isProvisioning(err) { + return Result{Status: "peer_provisioning", PeerID: peer.ID, PeerName: peer.Name}, nil + } if isGameNotFound(err) { return Result{Status: "peer_missing", PeerID: peer.ID, PeerName: peer.Name}, nil } diff --git a/internal/p2p/syncengine/hold.go b/internal/p2p/syncengine/hold.go index 7060c241..c318d7f4 100644 --- a/internal/p2p/syncengine/hold.go +++ b/internal/p2p/syncengine/hold.go @@ -64,6 +64,30 @@ const HeldMessage = "This device is holding this game back: its save files were // ErrHeld is SyncGame declining to sync a game that is held back. var ErrHeld = errors.New("its save files were all deleted here; it is not synced until you say whether that was meant") +// ProvisioningMessage is what this device says when a peer asks for a game +// that is still being configured. It must not contain "not found": that +// phrase is how the asker decides the peer does not track the game, and a +// provisioning hold is not an untrack. It is also distinct from HeldMessage, +// which means the save was emptied. +const ProvisioningMessage = "This device is still configuring this game and is not sending or receiving its saves yet" + +// ErrProvisioning is SyncGame declining to sync a game that is still being +// configured. It is not ErrHeld: that one is the emptied-save question. +var ErrProvisioning = errors.New("this game is still being configured and is not synced until that finishes") + +// ProvisioningUnreadableMessage is what this device says when it cannot tell +// whether a game is still being configured. It is not "not found" and it is +// not permission to send or receive the save. +const ProvisioningUnreadableMessage = "This device could not read whether this game is still being configured, so it is not sending or receiving its saves" + +// ErrProvisioningUnreadable is a failed hold lookup. It must not be treated +// as "not held". +var ErrProvisioningUnreadable = errors.New("could not read whether this game is still being configured") + +func isProvisioning(err error) bool { + return err != nil && strings.Contains(err.Error(), ProvisioningMessage) +} + // ErrNoHold is an answer to a question nobody asked: the game is not held. var ErrNoHold = errors.New("that game is not held back") diff --git a/internal/p2p/syncengine/provisioning_fault_test.go b/internal/p2p/syncengine/provisioning_fault_test.go new file mode 100644 index 00000000..cb80f184 --- /dev/null +++ b/internal/p2p/syncengine/provisioning_fault_test.go @@ -0,0 +1,55 @@ +package syncengine + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/opensave/opensave/internal/store" +) + +// A hold lookup that fails must not be treated as "not held". The game row +// is still readable, which is the shape that used to let SyncGame continue +// into FetchManifest. +func TestUnreadableProvisioningHoldDoesNotFetchManifest(t *testing.T) { + env := setupEngine(t) + if err := env.store.CreateHeldGame(store.Game{ + ID: "held1", Name: "Held", SavePath: env.localDir, MaxSnapshots: 10, + }); err != nil { + t.Fatal(err) + } + if _, err := env.store.GetGame("held1"); err != nil { + t.Fatal(err) + } + env.store.SetProvisioningReadFault(errors.New("injected hold read failure")) + if _, err := env.store.GetGame("held1"); err != nil { + t.Fatalf("fault leaked into game reads: %v", err) + } + + _, err := env.engine.SyncGame(context.Background(), "held1", []Peer{env.peer}) + if err == nil || !errors.Is(err, ErrProvisioningUnreadable) { + t.Fatalf("sync err = %v, want ErrProvisioningUnreadable", err) + } + if env.transport.manifestCalls != 0 { + t.Fatalf("FetchManifest was called %d times after a failed hold read", env.transport.manifestCalls) + } + if len(env.transport.deletedOnPeer) != 0 { + t.Fatalf("a failed hold read deleted %v", env.transport.deletedOnPeer) + } +} + +// An unpatched 2.4.1 client classifies a manifest error with these helpers. +// A provisioning refusal must not collapse into "not found", which is how a +// peer decides the save is gone. +func TestStockClientDoesNotTreatProvisioningRefusalAsMissing(t *testing.T) { + body := errors.New(`peer returned 409: {"error":"` + ProvisioningMessage + `"}`) + if isGameNotFound(body) || isHeld(body) || isAwaitingFolder(body) { + t.Fatalf("stock classifiers swallowed a provisioning refusal: notFound=%v held=%v awaiting=%v", + isGameNotFound(body), isHeld(body), isAwaitingFolder(body)) + } + unread := errors.New(`peer returned 503: {"error":"` + ProvisioningUnreadableMessage + `"}`) + if isGameNotFound(unread) || strings.Contains(strings.ToLower(unread.Error()), "not found") { + t.Fatal("an unreadable hold looks like the game is missing") + } +} diff --git a/internal/p2p/wanclient_handlers.go b/internal/p2p/wanclient_handlers.go index 95203dd3..77204852 100644 --- a/internal/p2p/wanclient_handlers.go +++ b/internal/p2p/wanclient_handlers.go @@ -530,7 +530,11 @@ func (w *WanClient) routeRequest(ctx context.Context, msg RelayMessage) (int, an return 200, map[string]any{"success": true} case route == "/games": - return 200, w.engine.PeerGameList() + list, err := w.engine.PeerGameList() + if err != nil { + return 503, map[string]string{"error": syncengine.ProvisioningUnreadableMessage} + } + return 200, list case strings.HasPrefix(route, "/manifest/"): return w.serveManifest(route, msg.Body, msg.From) @@ -555,7 +559,7 @@ func (w *WanClient) routeRequest(ctx context.Context, msg RelayMessage) (int, an w.engine.GoSync(func(ctx context.Context) { syncCtx, cancel := context.WithTimeout(ctx, 10*time.Minute) defer cancel() - if _, err := w.engine.SyncGame(syncCtx, gameID); err != nil && !errors.Is(err, syncengine.ErrPaused) && !errors.Is(err, syncengine.ErrHeld) { + if _, err := w.engine.SyncGame(syncCtx, gameID); err != nil && !errors.Is(err, syncengine.ErrPaused) && !errors.Is(err, syncengine.ErrHeld) && !errors.Is(err, syncengine.ErrProvisioning) { w.engine.Log("warn", fmt.Sprintf("WAN-triggered sync %s: %v", gameID, err)) } }) @@ -603,6 +607,9 @@ func (w *WanClient) serveManifest(route string, body json.RawMessage, peerID str if w.engine.holdingBack(game) { return 404, map[string]string{"error": syncengine.HeldMessage} } + if refuse, status, msg := w.engine.provisioningServeRefusal(game.ID); refuse { + return status, map[string]string{"error": msg} + } // As on the LAN: a save a sync here is writing is not described // (syncengine/settle.go). Each request runs on its own goroutine, so the // wait does not hold up the relay traffic that finishes the write. @@ -645,6 +652,9 @@ func (w *WanClient) serveBlocks(route string, rawBody json.RawMessage) (int, any if err != nil { return 404, map[string]string{"error": "Game not found."} } + if refuse, status, msg := w.engine.provisioningServeRefusal(game.ID); refuse { + return status, map[string]string{"error": msg} + } if !delta.IsSafePath(game.SavePath, body.RelPath) { return 403, map[string]string{"error": "Access denied: path traversal attempt detected."} } @@ -686,6 +696,9 @@ func (w *WanClient) serveDeleteFile(route string, rawBody json.RawMessage, fromP if err != nil { return 404, map[string]string{"error": "Game not found."} } + if refuse, status, msg := w.engine.provisioningServeRefusal(game.ID); refuse { + return status, map[string]string{"error": msg} + } if !delta.IsSafePath(game.SavePath, body.RelPath) { return 403, map[string]string{"error": "invalid path"} } @@ -732,6 +745,9 @@ func (w *WanClient) serveSnapshotDownload(route string) (int, any) { if err != nil { return 404, map[string]string{"error": "Snapshot ZIP file not found."} } + if refuse, status, msg := w.engine.provisioningServeRefusal(snap.GameID); refuse { + return status, map[string]string{"error": msg} + } archive, done, err := snapshot.OpenArchive(snap.ZipPath) if err != nil { return 404, map[string]string{"error": "Snapshot ZIP file not found."} diff --git a/internal/store/db.go b/internal/store/db.go index 9e69aeac..053bd83c 100644 --- a/internal/store/db.go +++ b/internal/store/db.go @@ -11,6 +11,7 @@ import ( "runtime" "sort" "strings" + "sync" "github.com/jmoiron/sqlx" _ "modernc.org/sqlite" @@ -24,6 +25,11 @@ var migrationsFS embed.FS // cloudtokens.go). type Store struct { db *sqlx.DB + // provisioningReadFault is nil in production. Tests set it so a hold + // lookup fails while the rest of the database still answers. A failed + // hold read must not be treated as "not held". + faultMu sync.Mutex + provisioningReadFault error } // Open creates (if needed) and opens the SQLite database at path, applying diff --git a/internal/store/games.go b/internal/store/games.go index 52d9c928..89dcb3bf 100644 --- a/internal/store/games.go +++ b/internal/store/games.go @@ -23,6 +23,17 @@ func SlugifyGameID(name string) string { return strings.Trim(id, "-") } +// ValidExplicitGameID reports whether id is a stable game id a caller may +// pin while provisioning. It has to be the same slug OpenSave would derive, +// so a paired device can be given the source's id without accepting a path +// or an arbitrary string. +func ValidExplicitGameID(id string) bool { + if id == "" || len(id) > 128 { + return false + } + return id == SlugifyGameID(id) +} + // ErrNotFound is returned by single-row lookups when no matching row exists. var ErrNotFound = errors.New("not found") @@ -73,6 +84,10 @@ type Game struct { // the thing that deletes an excluded config. SyncIgnore string `db:"sync_ignore" json:"syncIgnore"` CreatedAt string `db:"created_at" json:"createdAt"` + // ProvisioningHold asks TrackGame to record a provisioning hold with the + // game. It is not a column: the durable flag is game_provisioning_holds, + // so SELECT * into this struct stays valid for older and newer builds. + ProvisioningHold bool `db:"-" json:"-"` } // CreateGame inserts a new game and its default "main" branch in one diff --git a/internal/store/migrations/0038_game_provisioning_holds.sql b/internal/store/migrations/0038_game_provisioning_holds.sql new file mode 100644 index 00000000..8bd63c5f --- /dev/null +++ b/internal/store/migrations/0038_game_provisioning_holds.sql @@ -0,0 +1,16 @@ +-- A game being configured must not sync, even after a crash or a daemon +-- restart, and even if the other device has already started syncing it. +-- +-- This is not auto_sync = 0. That flag only stops this device from starting +-- a sync. A paired device that already has the game still asks for the +-- manifest and can pull or push files. Pause is in memory and dies with the +-- process. Neither is a place to leave a half-configured game. +-- +-- A table, not a column on games. Older builds read games with SELECT * and +-- a new column breaks them; a table they do not know is ignored. No row +-- means the game is not held, which is every game that existed before this +-- migration. +CREATE TABLE game_provisioning_holds ( + game_id TEXT PRIMARY KEY REFERENCES games(id) ON DELETE CASCADE, + held_at TEXT NOT NULL +); diff --git a/internal/store/provisioning.go b/internal/store/provisioning.go new file mode 100644 index 00000000..3d4cd27d --- /dev/null +++ b/internal/store/provisioning.go @@ -0,0 +1,192 @@ +package store + +import ( + "database/sql" + "errors" + "fmt" + "time" +) + +// Provisioning hold: a game that has been recorded but is not ready to sync. +// +// The row is written in the same transaction as the game, so a crash cannot +// leave a tracked game that the next launch will sync. Existing games have +// no row and are left alone. See migrations/0038_game_provisioning_holds.sql. + +// SetProvisioningReadFault makes later hold lookups fail with err while +// every other table still answers. Pass nil to clear it. Production never +// sets this; it exists so a test can prove a failed read is not "not held". +func (s *Store) SetProvisioningReadFault(err error) { + if s == nil { + return + } + s.faultMu.Lock() + s.provisioningReadFault = err + s.faultMu.Unlock() +} + +func (s *Store) provisioningFault() error { + if s == nil { + return fmt.Errorf("provisioning hold could not be read") + } + s.faultMu.Lock() + defer s.faultMu.Unlock() + return s.provisioningReadFault +} + +// ProvisioningBlocks reports whether gameID must not sync. An unknown id is +// not held. An alias of a held game is held. A failed read is an error, never +// a quiet false: callers must not serve, watch, or sync when this fails. +func (s *Store) ProvisioningBlocks(gameID string) (bool, error) { + if s == nil { + return false, fmt.Errorf("provisioning hold could not be read") + } + if gameID == "" { + return false, nil + } + if err := s.provisioningFault(); err != nil { + return false, err + } + held, err := s.ProvisioningHeld(gameID) + if err != nil || held { + return held, err + } + var canonical string + err = s.db.Get(&canonical, `SELECT game_id FROM game_aliases WHERE alias_id = ?`, gameID) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("provisioning hold alias %s: %w", gameID, err) + } + if canonical == "" || canonical == gameID { + return false, nil + } + return s.ProvisioningHeld(canonical) +} + +// ProvisioningHeld reports whether gameID is still being configured. +// An unknown id is not held. A database error is returned, not treated as false. +func (s *Store) ProvisioningHeld(gameID string) (bool, error) { + if s == nil { + return false, fmt.Errorf("provisioning hold could not be read") + } + if gameID == "" { + return false, nil + } + if err := s.provisioningFault(); err != nil { + return false, err + } + var one int + err := s.db.Get(&one, `SELECT 1 FROM game_provisioning_holds WHERE game_id = ?`, gameID) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("provisioning hold %s: %w", gameID, err) + } + return true, nil +} + +// ProvisioningHeldSet is every game currently held, for callers that walk +// the library and must not query once per game. +func (s *Store) ProvisioningHeldSet() (map[string]struct{}, error) { + if s == nil { + return nil, fmt.Errorf("provisioning hold could not be read") + } + if err := s.provisioningFault(); err != nil { + return nil, err + } + var ids []string + if err := s.db.Select(&ids, `SELECT game_id FROM game_provisioning_holds`); err != nil { + return nil, fmt.Errorf("list provisioning holds: %w", err) + } + out := make(map[string]struct{}, len(ids)) + for _, id := range ids { + out[id] = struct{}{} + } + return out, nil +} + +// CreateHeldGame inserts a game and its provisioning hold in one transaction. +// AutoSync is forced off. The local untrack tombstone is cleared in the same +// transaction: re-tracking is explicit, but it must not become visible to a +// peer before the hold exists. +func (s *Store) CreateHeldGame(g Game) error { + if g.ID == "" { + return fmt.Errorf("held game requires an id") + } + if g.ActiveBranch == "" { + g.ActiveBranch = "main" + } + if g.MaxSnapshots == 0 { + g.MaxSnapshots = 20 + } + g.AutoSync = false + + tx, err := s.db.Beginx() + if err != nil { + return err + } + defer tx.Rollback() + + if _, err := tx.NamedExec(` + INSERT INTO games (id, name, save_path, active_branch, auto_sync, max_snapshots, max_manual_snapshots, app_id, exe_path, cover_url) + VALUES (:id, :name, :save_path, :active_branch, :auto_sync, :max_snapshots, :max_manual_snapshots, :app_id, :exe_path, :cover_url)`, + g); err != nil { + return fmt.Errorf("insert game: %w", err) + } + if _, err := tx.Exec(`INSERT INTO branches (game_id, name) VALUES (?, ?)`, g.ID, g.ActiveBranch); err != nil { + return fmt.Errorf("insert default branch: %w", err) + } + if _, err := tx.Exec( + `INSERT INTO game_provisioning_holds (game_id, held_at) VALUES (?, ?)`, + g.ID, time.Now().UTC().Format(time.RFC3339Nano), + ); err != nil { + return fmt.Errorf("insert provisioning hold: %w", err) + } + if _, err := tx.Exec(`DELETE FROM untracked_games WHERE game_id = ?`, g.ID); err != nil { + return fmt.Errorf("clear tombstone: %w", err) + } + return tx.Commit() +} + +// ReleaseProvisioning clears the hold and turns AutoSync on, in one +// transaction. A game that is not held is left unchanged, including a game +// whose AutoSync the user turned off on purpose. The bool is whether this +// call released a hold. +func (s *Store) ReleaseProvisioning(gameID string) (bool, error) { + return s.ReleaseProvisioningMode(gameID, true) +} + +// ReleaseProvisioningMode clears the hold. enableAutoSync false leaves +// AutoSync off, so reconcile, reconnect and the watcher do not sync the game. +// The caller then syncs it explicitly. A game that is not held is left +// unchanged either way. +func (s *Store) ReleaseProvisioningMode(gameID string, enableAutoSync bool) (bool, error) { + tx, err := s.db.Beginx() + if err != nil { + return false, err + } + defer tx.Rollback() + + res, err := tx.Exec(`DELETE FROM game_provisioning_holds WHERE game_id = ?`, gameID) + if err != nil { + return false, fmt.Errorf("release provisioning hold %s: %w", gameID, err) + } + n, err := res.RowsAffected() + if err != nil { + return false, err + } + if n == 0 { + return false, tx.Commit() + } + auto := 0 + if enableAutoSync { + auto = 1 + } + if _, err := tx.Exec(`UPDATE games SET auto_sync = ? WHERE id = ?`, auto, gameID); err != nil { + return false, fmt.Errorf("set auto-sync %s: %w", gameID, err) + } + return true, tx.Commit() +} diff --git a/internal/store/provisioning_test.go b/internal/store/provisioning_test.go new file mode 100644 index 00000000..51fc34c1 --- /dev/null +++ b/internal/store/provisioning_test.go @@ -0,0 +1,149 @@ +package store + +import ( + "fmt" + "testing" +) + +func TestCreateHeldGameIsAtomicAndDefaultsExistingGamesClear(t *testing.T) { + s := openTestStore(t) + + plain := Game{ID: "already-live", Name: "Already Live", SavePath: "/tmp/live", ActiveBranch: "main", AutoSync: true, MaxSnapshots: 20} + if err := s.CreateGame(plain); err != nil { + t.Fatal(err) + } + held, err := s.ProvisioningHeld("already-live") + if err != nil { + t.Fatal(err) + } + if held { + t.Fatal("a game created without the hold was held; existing libraries must not change") + } + + if err := s.CreateHeldGame(Game{ + ID: "configuring", Name: "Configuring", SavePath: "/tmp/cfg", MaxSnapshots: 20, + AutoSync: true, // caller cannot override the forced off + }); err != nil { + t.Fatal(err) + } + got, err := s.GetGame("configuring") + if err != nil { + t.Fatal(err) + } + if got.AutoSync { + t.Fatal("held game was stored with AutoSync on") + } + held, err = s.ProvisioningHeld("configuring") + if err != nil || !held { + t.Fatalf("hold missing after commit: held=%v err=%v", held, err) + } + + set, err := s.ProvisioningHeldSet() + if err != nil { + t.Fatal(err) + } + if _, ok := set["configuring"]; !ok { + t.Fatal("held set omitted the new game") + } + if _, ok := set["already-live"]; ok { + t.Fatal("held set included a game that was not held") + } + + released, err := s.ReleaseProvisioning("already-live") + if err != nil { + t.Fatal(err) + } + if released { + t.Fatal("releasing a game that was not held reported a release") + } + plain, err = s.GetGame("already-live") + if err != nil { + t.Fatal(err) + } + if !plain.AutoSync { + t.Fatal("a no-op release changed AutoSync on a game that was not held") + } + + if err := s.CreateHeldGame(Game{ID: "staged", Name: "Staged", SavePath: "/tmp/staged", MaxSnapshots: 20}); err != nil { + t.Fatal(err) + } + released, err = s.ReleaseProvisioningMode("staged", false) + if err != nil || !released { + t.Fatalf("staged release = %v, %v", released, err) + } + staged, err := s.GetGame("staged") + if err != nil { + t.Fatal(err) + } + if staged.AutoSync { + t.Fatal("staged release turned AutoSync on") + } + + released, err = s.ReleaseProvisioning("configuring") + if err != nil || !released { + t.Fatalf("release = %v, %v", released, err) + } + got, err = s.GetGame("configuring") + if err != nil { + t.Fatal(err) + } + if !got.AutoSync { + t.Fatal("release left AutoSync off") + } + held, err = s.ProvisioningHeld("configuring") + if err != nil || held { + t.Fatalf("hold survived release: held=%v err=%v", held, err) + } + + released, err = s.ReleaseProvisioning("configuring") + if err != nil || released { + t.Fatalf("second release = %v, %v; want a no-op", released, err) + } +} + +func TestProvisioningReadFaultIsNotNotHeld(t *testing.T) { + s := openTestStore(t) + if err := s.CreateHeldGame(Game{ID: "held-game", Name: "Held", SavePath: "/tmp/held", MaxSnapshots: 20}); err != nil { + t.Fatal(err) + } + if err := s.AddGameAlias("alias-of-held", "held-game"); err != nil { + t.Fatal(err) + } + blocks, err := s.ProvisioningBlocks("alias-of-held") + if err != nil || !blocks { + t.Fatalf("alias of a held game blocks=%v err=%v", blocks, err) + } + blocks, err = s.ProvisioningBlocks("no-such-game") + if err != nil || blocks { + t.Fatalf("unknown id blocks=%v err=%v; want not held", blocks, err) + } + + s.SetProvisioningReadFault(fmt.Errorf("injected hold read failure")) + blocks, err = s.ProvisioningBlocks("held-game") + if err == nil || blocks { + t.Fatalf("faulty read blocks=%v err=%v; a failed read must be an error, not held=false", blocks, err) + } + if _, err := s.ProvisioningHeldSet(); err == nil { + t.Fatal("held set succeeded while reads were faulted") + } + // The game row itself is still readable. That is the fail-open shape: + // the hold query fails and a caller that ignores the error would sync. + if _, err := s.GetGame("held-game"); err != nil { + t.Fatalf("fault leaked into game reads: %v", err) + } +} + +func TestValidExplicitGameID(t *testing.T) { + ok := []string{"hades", "crash-bandicoot-4", "a1"} + bad := []string{"", "Hades", "../x", "a/b", "a b", "a--b", "-a", "a-"} + for _, id := range ok { + if !ValidExplicitGameID(id) { + t.Errorf("rejected %q", id) + } + } + for _, id := range bad { + if ValidExplicitGameID(id) { + t.Errorf("accepted %q", id) + } + } +} diff --git a/testutil/harness.go b/testutil/harness.go index 9323e7b4..0427a3ab 100644 --- a/testutil/harness.go +++ b/testutil/harness.go @@ -176,12 +176,59 @@ func NewTestDaemon(t *testing.T, name string) *TestDaemon { td := &TestDaemon{T: t, Daemon: d, Server: srv, Addr: addr, Port: port, SaveDir: saveDir} t.Cleanup(func() { - srv.Stop() - d.Stop() + td.Stop() }) return td } +// Stop shuts down whatever daemon this handle currently points at. +func (td *TestDaemon) Stop() { + if td.Server != nil { + td.Server.Stop() + td.Server = nil + } + if td.Daemon != nil { + td.Daemon.Stop() + td.Daemon = nil + } +} + +// Restart replaces the process the way an unmanaged `opensave daemon start` +// does: daemon.New + Start on the same data directory and the same port. +// In-memory pause does not survive. The persisted store does. +func (td *TestDaemon) Restart() { + td.T.Helper() + home := td.Daemon.Paths.HomeDir + port := td.Port + saveDir := td.SaveDir + td.Stop() + + d, err := daemon.New(daemon.Options{HomeOverride: home, DisableDiscovery: true}) + if err != nil { + td.T.Fatalf("restart daemon.New: %v", err) + } + d.Scanner.ManifestURL = "" + d.Scanner.SteamUserdataPaths = []string{} + d.Scanner.SteamRoots = []string{} + d.Scanner.LocalLowDir = filepath.Join(home, "no-locallow") + d.Scanner.ResolveAppName = nil + if err := d.Start(); err != nil { + d.Stop() + td.T.Fatalf("restart daemon.Start: %v", err) + } + srv := api.New(d) + addr, err := srv.Start(port) + if err != nil { + d.Stop() + td.T.Fatalf("restart api.Start: %v", err) + } + td.Daemon = d + td.Server = srv + td.Addr = addr + td.Port = port + td.SaveDir = saveDir +} + // API performs a JSON request against this daemon's API and decodes the // response into out (out may be nil). Non-2xx responses fail the test // unless allowError is set.