fix(datastore): fsync atomicWriteFile for crash-safe durability (#458)

atomicWriteFile wrote a temp file and renamed it, but never fsync'd — so an
unclean power-cut on a journaling NAND filesystem (UBIFS on the speaker's
/mnt/nv) could leave the renamed datastore file present but 0 bytes (the rename
was journalled, the data blocks were not flushed). Now fsync the temp file
before the rename and the parent directory after, via os.Root.OpenFile/Open;
directory fsync is best-effort (unsupported on some filesystems).

Pairs with the read-side resilience fix (#459): durability prevents the 0-byte
files; resilience tolerates any that already exist.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pkg/service/datastore/atomic_write_test.go

@@ -0,0 +1,52 @@
+package datastore
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// TestAtomicWriteFile_DurableRoundTrip guards the durable write path added for
+// #458: content must round-trip, an overwrite must truncate cleanly, and no
+// `.tmp` sidecar may be left behind. (The fsync durability itself isn't
+// unit-testable without power-loss fault injection; this is the functional
+// regression guard so the fsync rework doesn't break writes.)
+func TestAtomicWriteFile_DurableRoundTrip(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "st-atomic-test-*")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Cleanup(func() { _ = os.RemoveAll(tempDir) })
+
+	ds := NewDataStore(tempDir)
+
+	dir := ds.AccountDeviceDir("1234567", "001122334455")
+	if err := os.MkdirAll(dir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	path := filepath.Join(dir, "Sources.xml")
+
+	if err := ds.atomicWriteFile(path, []byte("first")); err != nil {
+		t.Fatalf("atomicWriteFile (create) failed: %v", err)
+	}
+
+	if got, _ := os.ReadFile(path); string(got) != "first" {
+		t.Errorf("content after create = %q, want %q", got, "first")
+	}
+
+	// Overwrite must truncate the previous (longer) content, not leave a tail.
+	if err := ds.atomicWriteFile(path, []byte("hi")); err != nil {
+		t.Fatalf("atomicWriteFile (overwrite) failed: %v", err)
+	}
+
+	if got, _ := os.ReadFile(path); string(got) != "hi" {
+		t.Errorf("content after overwrite = %q, want %q", got, "hi")
+	}
+
+	// No leftover temp sidecar.
+	if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) {
+		t.Errorf("expected no %s.tmp leftover, stat err = %v", path, err)
+	}
+}

pkg/service/datastore/datastore.go

@@ -1210,15 +1210,83 @@ func (ds *DataStore) SavePresets(account, device string, presets []models.Servic
 	return ds.atomicWriteFile(path, append(header, data...))
 }
 
+// atomicWriteFile writes data to filename atomically AND durably: it writes a
+// temp file, fsyncs it, renames it into place, then fsyncs the parent
+// directory. Without the fsyncs an unclean power-cut on a journaling NAND
+// filesystem (UBIFS, the speaker's /mnt/nv) can leave the renamed file present
+// but 0 bytes — the rename was journalled but the data blocks were never
+// flushed. See #458.
 func (ds *DataStore) atomicWriteFile(filename string, data []byte) error {
 	perm := os.FileMode(0644)
 
 	tempFile := filename + ".tmp"
-	if err := ds.rootWriteFile(tempFile, data, perm); err != nil {
+	if err := ds.rootWriteFileSync(tempFile, data, perm); err != nil {
 		return err
 	}
 
-	return ds.rootRename(tempFile, filename)
+	if err := ds.rootRename(tempFile, filename); err != nil {
+		return err
+	}
+
+	// Fsync the parent directory so the rename itself survives a power-cut.
+	// Best-effort: not every filesystem permits directory fsync, and the data +
+	// rename have already succeeded by this point.
+	ds.rootSyncDir(filepath.Dir(filename))
+
+	return nil
+}
+
+// rootWriteFileSync writes data to absPath (truncating any existing file) and
+// fsyncs the file before returning, so the contents are on stable storage. This
+// is the durable equivalent of rootWriteFile.
+func (ds *DataStore) rootWriteFileSync(absPath string, data []byte, perm os.FileMode) error {
+	r, err := ds.getRoot()
+	if err != nil {
+		return err
+	}
+
+	rel, err := ds.rootRel(absPath)
+	if err != nil {
+		return err
+	}
+
+	f, err := r.OpenFile(rel, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
+	if err != nil {
+		return err
+	}
+
+	if _, werr := f.Write(data); werr != nil {
+		_ = f.Close()
+
+		return werr
+	}
+
+	if serr := f.Sync(); serr != nil {
+		_ = f.Close()
+
+		return serr
+	}
+
+	return f.Close()
+}
+
+// rootSyncDir fsyncs the directory at absDir so a preceding create/rename is
+// durable. Best-effort: directory fsync isn't supported on every filesystem, so
+// failures are logged and swallowed rather than failing an already-successful
+// write.
+func (ds *DataStore) rootSyncDir(absDir string) {
+	d, err := ds.rootOpen(absDir)
+	if err != nil {
+		log.Printf("[Datastore] rootSyncDir: open %s failed (best-effort): %v", sanitizeLog(absDir), err)
+
+		return
+	}
+
+	if serr := d.Sync(); serr != nil {
+		log.Printf("[Datastore] rootSyncDir: fsync %s failed (best-effort): %v", sanitizeLog(absDir), serr)
+	}
+
+	_ = d.Close()
 }
 
 // GetRecents returns the list of recently played items for the specified account and device.