fix(persist): fsync before rename so a crash can't lose the latest write

Session and memory writes did WriteFile(tmp)+Rename — atomic against torn
reads, but NOT durable: without an fsync a power loss / kernel crash can
land the rename while the data is still in the page cache, leaving an empty
or truncated file and silently losing the latest conversation turn or
extracted memory. session.go also used a fixed "<target>.tmp" name, so two
concurrent saves of the same target could clobber each other's temp file.

Add internal/fsatomic.WriteFile (unique temp → fsync data → rename → fsync
dir) and route the irreplaceable writes through it: session save + index,
episode index + summaries, and the facts store.

Tests: fsatomic content/perm/overwrite, no temp litter, and concurrent
same-target writers never producing a torn file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jkyberneees

internal/fsatomic/fsatomic.go

@@ -0,0 +1,70 @@
+// Package fsatomic provides a crash-durable atomic file write.
+//
+// The common "write a temp file then rename over the target" idiom gives
+// atomicity (a reader sees either the old or the new file, never a torn one),
+// but NOT durability: without an fsync, a power loss or kernel crash can land
+// the rename in the directory while the file's data is still only in the page
+// cache, leaving an empty or truncated file after reboot. For data the agent
+// can't reconstruct — conversation sessions, extracted memories — that is silent
+// data loss.
+//
+// WriteFile closes the gap: it fsyncs the file data before the rename and
+// fsyncs the parent directory after, so a successful return means the bytes are
+// durably on disk. It also uses a unique temp name, so two concurrent writers
+// to the same target can't clobber each other's temp file.
+package fsatomic
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// WriteFile atomically and durably writes data to path with the given perm.
+// On success the bytes are fsynced to disk and the rename is durable.
+func WriteFile(path string, data []byte, perm os.FileMode) (err error) {
+	dir := filepath.Dir(path)
+
+	f, err := os.CreateTemp(dir, "."+filepath.Base(path)+".tmp-*")
+	if err != nil {
+		return fmt.Errorf("fsatomic: create temp: %w", err)
+	}
+	tmp := f.Name()
+	// Remove the temp file on any failure before the rename succeeds.
+	defer func() {
+		if tmp != "" {
+			os.Remove(tmp)
+		}
+	}()
+
+	if _, err := f.Write(data); err != nil {
+		f.Close()
+		return fmt.Errorf("fsatomic: write: %w", err)
+	}
+	if err := f.Chmod(perm); err != nil {
+		f.Close()
+		return fmt.Errorf("fsatomic: chmod: %w", err)
+	}
+	// Flush the file's data to disk before exposing it via the rename.
+	if err := f.Sync(); err != nil {
+		f.Close()
+		return fmt.Errorf("fsatomic: fsync temp: %w", err)
+	}
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("fsatomic: close temp: %w", err)
+	}
+
+	if err := os.Rename(tmp, path); err != nil {
+		return fmt.Errorf("fsatomic: rename: %w", err)
+	}
+	tmp = "" // renamed — no longer ours to remove
+
+	// Make the rename itself durable. Best-effort: some filesystems don't
+	// support directory fsync, and the data is already synced, so a failure
+	// here doesn't corrupt anything — it just weakens the crash guarantee.
+	if d, derr := os.Open(dir); derr == nil {
+		_ = d.Sync()
+		d.Close()
+	}
+	return nil
+}

internal/fsatomic/fsatomic_test.go

@@ -0,0 +1,111 @@
+package fsatomic
+
+import (
+	"os"
+	"path/filepath"
+	"sync"
+	"testing"
+)
+
+func TestWriteFile_WritesContentAndPerm(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "data.json")
+
+	if err := WriteFile(path, []byte("hello"), 0600); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	got, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile: %v", err)
+	}
+	if string(got) != "hello" {
+		t.Errorf("content = %q, want %q", got, "hello")
+	}
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Errorf("perm = %v, want 0600", info.Mode().Perm())
+	}
+}
+
+func TestWriteFile_Overwrites(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "data")
+	if err := WriteFile(path, []byte("first"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteFile(path, []byte("second"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	got, _ := os.ReadFile(path)
+	if string(got) != "second" {
+		t.Errorf("content = %q, want %q", got, "second")
+	}
+}
+
+// TestWriteFile_LeavesNoTempOnSuccess verifies the unique temp file is renamed
+// away (no litter), which also confirms the temp naming doesn't collide with
+// the target.
+func TestWriteFile_LeavesNoTempOnSuccess(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "data")
+	if err := WriteFile(path, []byte("x"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	entries, _ := os.ReadDir(dir)
+	if len(entries) != 1 || entries[0].Name() != "data" {
+		var names []string
+		for _, e := range entries {
+			names = append(names, e.Name())
+		}
+		t.Errorf("expected only [data], got %v", names)
+	}
+}
+
+// TestWriteFile_ConcurrentSameTarget verifies concurrent writers to the same
+// path don't clobber each other's temp file (the old fixed-".tmp" pattern
+// could) — the final content must be a complete one of the writes, never torn.
+func TestWriteFile_ConcurrentSameTarget(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "data")
+	payloads := []string{
+		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+		"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+		"cccccccccccccccccccccccccccccc",
+	}
+	var wg sync.WaitGroup
+	for _, p := range payloads {
+		wg.Add(1)
+		go func(data string) {
+			defer wg.Done()
+			for i := 0; i < 20; i++ {
+				if err := WriteFile(path, []byte(data), 0644); err != nil {
+					t.Errorf("WriteFile: %v", err)
+					return
+				}
+			}
+		}(p)
+	}
+	wg.Wait()
+
+	got, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ok := false
+	for _, p := range payloads {
+		if string(got) == p {
+			ok = true
+		}
+	}
+	if !ok {
+		t.Errorf("final content %q is torn — not a complete write", got)
+	}
+	// No temp files left behind.
+	entries, _ := os.ReadDir(dir)
+	if len(entries) != 1 {
+		t.Errorf("expected only the target file, got %d entries", len(entries))
+	}
+}

internal/memory/episodes.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/BackendStack21/go-vector/pkg/vector"
 	"github.com/BackendStack21/odek/internal/embedding"
+	"github.com/BackendStack21/odek/internal/fsatomic"
 	"github.com/BackendStack21/odek/internal/session"
 )
 
@@ -224,7 +225,7 @@ func (e *EpisodeStore) writeLocked(sessionID, summary string, turns int, prov Ep
 
 	// Write the summary file.
 	path := filepath.Join(e.dir, sessionID+".md")
-	if err := os.WriteFile(path, []byte(summary), 0600); err != nil {
+	if err := fsatomic.WriteFile(path, []byte(summary), 0600); err != nil {
 		return events, fmt.Errorf("memory: write episode: %w", err)
 	}
 
@@ -661,19 +662,14 @@ func trustRank(p EpisodeProvenance) int {
 // Invalidates the in-memory cache after a successful write so the next
 // ReadIndex call picks up the new data.
 func (e *EpisodeStore) writeIndex(idx []EpisodeMeta) error {
-	// Write to temp + rename for atomicity
+	// Write atomically and durably (temp → fsync → rename → dir fsync).
 	idxPath := filepath.Join(e.dir, episodeIndexFile)
-	tmpPath := idxPath + ".tmp"
 
 	data, err := json.MarshalIndent(idx, "", "  ")
 	if err != nil {
 		return fmt.Errorf("memory: marshal index: %w", err)
 	}
-	if err := os.WriteFile(tmpPath, data, 0600); err != nil {
-		return err
-	}
-	if err := os.Rename(tmpPath, idxPath); err != nil {
-		os.Remove(tmpPath) // best-effort cleanup
+	if err := fsatomic.WriteFile(idxPath, data, 0600); err != nil {
 		return err
 	}
 

internal/memory/facts.go

@@ -27,6 +27,8 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+
+	"github.com/BackendStack21/odek/internal/fsatomic"
 )
 
 // File names for fact targets.
@@ -313,27 +315,7 @@ func (f *FactStore) Entries(target string) ([]string, error) {
 // mutual exclusion that f.mu provides per-instance.
 func (f *FactStore) writeEntries(target string, entries []string) error {
 	content := strings.Join(entries, entrySep)
-	path := f.path(target)
-
-	tmp, err := os.CreateTemp(f.dir, filepath.Base(path)+".tmp-*")
-	if err != nil {
-		return err
-	}
-	tmpName := tmp.Name()
-	if _, err := tmp.Write([]byte(content)); err != nil {
-		tmp.Close()
-		os.Remove(tmpName)
-		return err
-	}
-	if err := tmp.Close(); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	return nil
+	return fsatomic.WriteFile(f.path(target), []byte(content), 0600)
 }
 
 // parseEntries splits file content into individual entries.

internal/session/session.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/BackendStack21/odek/internal/embedding"
+	"github.com/BackendStack21/odek/internal/fsatomic"
 	"github.com/BackendStack21/odek/internal/llm"
 	"github.com/BackendStack21/odek/internal/redact"
 )
@@ -197,15 +198,8 @@ func (s *Store) saveIndexLocked(idx map[string]*IndexEntry) error {
 	if err != nil {
 		return fmt.Errorf("session: marshal index: %w", err)
 	}
-	target := s.indexPath()
-	tmp := target + ".tmp"
-	if err := os.WriteFile(tmp, data, 0600); err != nil {
-		os.Remove(tmp)
-		return fmt.Errorf("session: write index tmp: %w", err)
-	}
-	if err := os.Rename(tmp, target); err != nil {
-		os.Remove(tmp)
-		return fmt.Errorf("session: rename index: %w", err)
+	if err := fsatomic.WriteFile(s.indexPath(), data, 0600); err != nil {
+		return fmt.Errorf("session: write index: %w", err)
 	}
 	return nil
 }
@@ -265,9 +259,10 @@ func (s *Store) Append(id string, newMsgs []llm.Message) error {
 	return s.saveLocked(sess)
 }
 
-// Save writes a session to disk atomically using a temp-file + rename
-// strategy. This prevents:
+// Save writes a session to disk atomically and durably via fsatomic.WriteFile
+// (temp-file → fsync → rename → dir fsync). This prevents:
 //   - Partial writes from crashes (rename is atomic on POSIX)
+//   - Data loss on power failure (the fsync flushes bytes before the rename)
 //   - Symlink-following TOCTOU attacks (os.Rename replaces the
 //     directory entry itself — it does NOT follow symlinks)
 func (s *Store) Save(sess *Session) error {
@@ -297,15 +292,8 @@ func (s *Store) saveLocked(sess *Session) error {
 		return fmt.Errorf("session: marshal: %w", err)
 	}
 
-	target := s.path(sess.ID)
-	tmp := target + ".tmp"
-	if err := os.WriteFile(tmp, data, 0600); err != nil {
-		os.Remove(tmp)
-		return fmt.Errorf("session: write tmp: %w", err)
-	}
-	if err := os.Rename(tmp, target); err != nil {
-		os.Remove(tmp)
-		return fmt.Errorf("session: rename: %w", err)
+	if err := fsatomic.WriteFile(s.path(sess.ID), data, 0600); err != nil {
+		return fmt.Errorf("session: write: %w", err)
 	}
 
 	// Update the index atomically.