watcher: a watch stopped during its snapshot starts no sync

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()

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)
+	}
+}

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)
 	}