From 9c2dde30ab834edc4b1188e04efaed289eb19c76 Mon Sep 17 00:00:00 2001
From: ChakraFusion <112588066+ChakraFusion@users.noreply.github.com>
Date: Wed, 14 Oct 2026 14:15:22 +0200
Subject: [PATCH] watcher: a watch stopped during its snapshot starts no sync

A snapshot can take longer than stopping waits for its watch
(watchStopTimeout, 5 s). The watch was given up, but its handleChange ran
on, finished the snapshot and called OnChanged, which syncs the game - a
game whose auto-sync had just been switched off.

Seen in the logs #57 printed for TestMultiRootScenario_ALateReportDoes
NotHideADivergence on Windows CI: "the watcher for loclatereport did not
stop within 5s and was abandoned", then "auto-snapshot created" and a sync
with the other device, whose "in sync" report gave it a fresh last-synced
time just before the test's edits.

- handleChange keeps the snapshot, which is the save as it changed, but
  announces nothing once its watch was stopped.
- The daemon's OnChanged starts no sync for a game whose auto-sync is off,
  which also covers the catch-up, run outside any one watch.

Test: TestAWatchStoppedDuringItsSnapshotAnnouncesNothing holds a snapshot
mid-way and stops the watch (fails without the change: the change is
announced after the stop).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/daemon/daemon.go              | 11 +++++
 internal/watcher/stopped_watch_test.go | 65 ++++++++++++++++++++++++++
 internal/watcher/watcher.go            |  9 ++++
 3 files changed, 85 insertions(+)
 create mode 100644 internal/watcher/stopped_watch_test.go

diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go
index 306ce6ff..cf622cd1 100644
--- a/internal/daemon/daemon.go
+++ b/internal/daemon/daemon.go
@@ -236,6 +236,17 @@ func New(opts Options) (*Daemon, error) {
 			// Watcher-detected save change: push it to online peers. Bound to
 			// the P2P engine's lifecycle so shutdown cancels a transfer in
 			// flight instead of leaving it writing into the save folder.
+			//
+			// Not for a game whose auto-sync is off: a change reported after
+			// it was switched off — by the catch-up, or a watch that finished
+			// its snapshot as it was being stopped — is the game's, but syncing
+			// it is what the setting says not to do.
+			if game, err := d.Store.GetGame(gameID); err != nil || !game.AutoSync {
+				if d.OnGameChanged != nil {
+					d.OnGameChanged(gameID)
+				}
+				return
+			}
 			d.P2P.GoSync(func(ctx context.Context) {
 				ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
 				defer cancel()
diff --git a/internal/watcher/stopped_watch_test.go b/internal/watcher/stopped_watch_test.go
new file mode 100644
index 00000000..992b170b
--- /dev/null
+++ b/internal/watcher/stopped_watch_test.go
@@ -0,0 +1,65 @@
+package watcher
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// A watch stopped while its snapshot is being taken keeps the snapshot and
+// announces nothing.
+//
+// A snapshot can take longer than stopping waits for the watch, so it finished
+// after the watch had been given up, and announced the change: the daemon then
+// synced a game whose auto-sync had just been switched off. On Windows CI that
+// was the sync nobody expected in
+// TestMultiRootScenario_ALateReportDoesNotHideADivergence ("the watcher ...
+// did not stop within 5s and was abandoned", then "auto-snapshot created" and
+// a sync).
+func TestAWatchStoppedDuringItsSnapshotAnnouncesNothing(t *testing.T) {
+	saveDir := t.TempDir()
+	col := newCollector()
+	cb := col.callbacks()
+	inSnapshot := make(chan struct{}, 1)
+	release := make(chan struct{})
+	take := cb.CreateSnapshot
+	cb.CreateSnapshot = func(gameID string) error {
+		select {
+		case inSnapshot <- struct{}{}:
+		default:
+		}
+		<-release
+		return take(gameID)
+	}
+	eng := New(cb)
+	defer eng.Stop()
+	if err := eng.Watch("game1", saveDir); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(saveDir, "slot1.sav"), []byte("played"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	select {
+	case <-inSnapshot:
+	case <-time.After(15 * time.Second):
+		t.Fatal("the change was never snapshotted")
+	}
+
+	stopped := make(chan struct{})
+	go func() {
+		eng.Unwatch("game1")
+		close(stopped)
+	}()
+	time.Sleep(300 * time.Millisecond) // the watch is cancelled; its snapshot still runs
+	close(release)
+	<-stopped
+	time.Sleep(300 * time.Millisecond)
+
+	if n := col.snapshotCount(); n != 1 {
+		t.Errorf("%d snapshots, want the one being taken kept", n)
+	}
+	if n := col.changedCount(); n != 0 {
+		t.Errorf("the change was announced %d time(s) after its watch was stopped", n)
+	}
+}
diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go
index 612ebfb6..3e81268f 100644
--- a/internal/watcher/watcher.go
+++ b/internal/watcher/watcher.go
@@ -844,6 +844,15 @@ func (e *Engine) handleChange(ctx context.Context, gw *gameWatch) {
 	}
 	e.log("success", fmt.Sprintf("auto-snapshot created for %q", gw.gameID))
 
+	// Stopped while the snapshot was taken — auto-sync switched off, the game
+	// untracked, the app quitting: the snapshot is kept, as it is the save as
+	// it changed, but nothing more happens on behalf of a watch nobody wants.
+	// A snapshot can take seconds, longer than stopping waits (stop), so this
+	// ran on after the watch was given up and started a sync of a game whose
+	// auto-sync had just been switched off.
+	if ctx.Err() != nil {
+		return
+	}
 	if e.cb.OnChanged != nil {
 		e.cb.OnChanged(gw.gameID)
 	}