fix(storage): stop compaction from destroying the log on Windows

pkg/storage/hybrid/store.go

@@ -131,6 +131,11 @@ func New(config Config) (*Store, error) {
 	logPath := config.DataPath
 	var err error
 
+	// Adopt an orphaned compaction file left by an interrupted swap. Without
+	// this, openAppendLog's O_CREATE silently produces an empty log and the
+	// data in the .compact file is lost for good.
+	recoverOrphanedCompaction(logPath)
+
 	s.log, err = openAppendLog(logPath)
 	if err != nil {
 		cancel()
@@ -583,25 +588,101 @@ func (s *Store) Compact() error {
 		return fmt.Errorf("failed to sync compaction log: %w", err)
 	}
 
-	// Swap logs
+	// Swap logs.
+	//
+	// Both files must be closed first. Go opens files without
+	// FILE_SHARE_DELETE on Windows, so a rename fails while either the source
+	// or the destination is still open - which made the previous
+	// remove-then-rename sequence delete the log and then reliably fail to put
+	// the compacted copy in its place.
+	//
+	// The old log is also never removed up front: os.Rename replaces an
+	// existing destination on POSIX and Windows alike, so removing it
+	// beforehand only creates a window where a failed rename loses everything.
 	oldLog := s.log
 	oldPath := oldLog.path
 
-	s.log = newLog
+	if err := newLog.Close(); err != nil {
+		_ = os.Remove(newLogPath)
+		return fmt.Errorf("failed to close compaction log: %w", err)
+	}
+	if err := oldLog.Close(); err != nil {
+		_ = os.Remove(newLogPath)
+		return fmt.Errorf("failed to close log before compaction swap: %w", err)
+	}
+
+	if err := renameWithRetry(newLogPath, oldPath); err != nil {
+		// The old log is untouched on disk - reopen it so the store keeps
+		// serving the pre-compaction state instead of pointing at nothing.
+		reopened, reopenErr := openAppendLog(oldPath)
+		if reopenErr != nil {
+			return fmt.Errorf("failed to swap compacted log into place (%w) and could not reopen the original log: %w", err, reopenErr)
+		}
+		s.log = reopened
+		return fmt.Errorf("failed to swap compacted log into place: %w", err)
+	}
+
+	swapped, err := openAppendLog(oldPath)
+	if err != nil {
+		return fmt.Errorf("failed to reopen compacted log: %w", err)
+	}
+
+	s.log = swapped
 	s.index = newIndex
 	s.cache.Clear()
 
-	// Close and remove old log
-	_ = oldLog.Close()
-	_ = os.Remove(oldPath)
-	_ = os.Rename(newLogPath, oldPath)
-	s.log.path = oldPath
-
 	s.stats.Compactions.Add(1)
 
 	return nil
 }
 
+// recoverOrphanedCompaction adopts a leftover ".compact" file when the log it
+// was meant to replace is missing or empty.
+//
+// Earlier versions removed the log before renaming the compacted copy over it,
+// so an interrupted swap left only the ".compact" file behind. openAppendLog
+// creates the missing log with O_CREATE, so without this the store starts up
+// empty and the surviving data is discarded on the next write. Adopting the
+// ".compact" file is safe: it is written and synced in full before the swap is
+// attempted, and Compact() holds an exclusive lock, so it is never a partial
+// snapshot of a concurrent compaction.
+func recoverOrphanedCompaction(logPath string) {
+	compactPath := logPath + ".compact"
+
+	compactInfo, err := os.Stat(compactPath)
+	if err != nil || compactInfo.Size() <= logHeaderSize {
+		return // nothing usable to recover
+	}
+
+	// Only step in when the real log cannot be the newer copy.
+	if info, err := os.Stat(logPath); err == nil && info.Size() > logHeaderSize {
+		return
+	}
+
+	_ = renameWithRetry(compactPath, logPath)
+}
+
+// renameWithRetry renames oldPath to newPath, retrying briefly on failure.
+//
+// On Windows a rename fails while any other process holds the destination
+// open - antivirus, search indexers and backup agents all do this transiently.
+// The Go toolchain retries renames in the module cache for the same reason
+// (golang/go#37802). A few short retries turn a hard failure into a pause.
+func renameWithRetry(oldPath, newPath string) error {
+	const attempts = 5
+
+	var err error
+	for i := range attempts {
+		if err = os.Rename(oldPath, newPath); err == nil {
+			return nil
+		}
+		if i < attempts-1 {
+			time.Sleep(time.Duration(50<<i) * time.Millisecond)
+		}
+	}
+	return err
+}
+
 func (s *Store) GetStats() StatsMeta {
 	return StatsMeta{
 		Writes:      s.stats.Writes.Load(),

pkg/storage/hybrid/store_compaction_test.go

@@ -0,0 +1,161 @@
+package hybrid
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/sirrobot01/decypharr/internal/config"
+)
+
+// The package logger loads the global config on first use, which writes a
+// config file. Point it at a scratch directory so tests never touch a real one.
+func TestMain(m *testing.M) {
+	dir, err := os.MkdirTemp("", "decypharr-hybrid-test")
+	if err != nil {
+		panic(err)
+	}
+	config.SetConfigPath(dir)
+	code := m.Run()
+	_ = os.RemoveAll(dir)
+	os.Exit(code)
+}
+
+func testStore(t *testing.T, path string) *Store {
+	t.Helper()
+	s, err := New(Config{DataPath: path, CacheSize: 16})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	return s
+}
+
+// churn writes and deletes enough entries that the log is mostly dead space,
+// which is what makes compaction worth doing.
+func churn(t *testing.T, s *Store) {
+	t.Helper()
+	for i := range 32 {
+		key := string(rune('a' + i%26))
+		if err := s.Put(key, []byte("value-that-takes-up-some-room"), nil); err != nil {
+			t.Fatalf("Put: %v", err)
+		}
+	}
+	for i := range 16 {
+		if err := s.Delete(string(rune('a' + i))); err != nil {
+			t.Fatalf("Delete: %v", err)
+		}
+	}
+}
+
+// TestCompactKeepsLogWhenSwapFails is the regression test for the compaction
+// swap destroying the store. The old code removed the log and then renamed the
+// compacted copy over it, discarding both errors, so a failed rename left no
+// log at all and every later read returned EOF.
+//
+// The failure is forced by making the destination un-renameable: on Windows an
+// open handle is enough, and everywhere else a directory in place of the file
+// makes the rename fail. Either way the store must survive with its data.
+func TestCompactKeepsLogWhenSwapFails(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.db")
+
+	s := testStore(t, path)
+	defer s.Close()
+	churn(t, s)
+	if err := s.Put("keeper", []byte("must-survive"), nil); err != nil {
+		t.Fatalf("Put keeper: %v", err)
+	}
+
+	// Hold the destination open so the rename cannot replace it.
+	blocker, err := os.OpenFile(path, os.O_RDWR, 0644)
+	if err != nil {
+		t.Fatalf("open blocker: %v", err)
+	}
+
+	compactErr := s.Compact()
+	blocker.Close()
+
+	// The rename genuinely fails only on Windows; elsewhere this compaction
+	// succeeds and there is nothing to assert about the failure path.
+	if compactErr == nil {
+		t.Skip("rename over an open file succeeded on this platform")
+	}
+
+	if _, err := os.Stat(path); err != nil {
+		t.Fatalf("log must still exist after a failed compaction swap: %v", err)
+	}
+	got, err := s.Get("keeper")
+	if err != nil {
+		t.Fatalf("store must keep serving after a failed swap: %v", err)
+	}
+	if string(got) != "must-survive" {
+		t.Fatalf("got %q, want %q", got, "must-survive")
+	}
+}
+
+// TestCompactSucceedsAndLeavesNoTempFile covers the normal path: the data
+// survives, the log is at the expected path, and no ".compact" file is left.
+func TestCompactSucceedsAndLeavesNoTempFile(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.db")
+
+	s := testStore(t, path)
+	defer s.Close()
+	churn(t, s)
+	if err := s.Put("keeper", []byte("must-survive"), nil); err != nil {
+		t.Fatalf("Put keeper: %v", err)
+	}
+
+	if err := s.Compact(); err != nil {
+		t.Fatalf("Compact: %v", err)
+	}
+
+	if _, err := os.Stat(path + ".compact"); !os.IsNotExist(err) {
+		t.Fatalf("compaction temp file must not be left behind")
+	}
+	if s.log.path != path {
+		t.Fatalf("log path = %q, want %q", s.log.path, path)
+	}
+	got, err := s.Get("keeper")
+	if err != nil {
+		t.Fatalf("Get after compaction: %v", err)
+	}
+	if string(got) != "must-survive" {
+		t.Fatalf("got %q, want %q", got, "must-survive")
+	}
+}
+
+// TestRecoverOrphanedCompaction covers stores already broken by the old code:
+// only a ".compact" file survives, and the store must adopt it instead of
+// silently starting empty (openAppendLog creates the missing log with O_CREATE).
+func TestRecoverOrphanedCompaction(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.db")
+
+	s := testStore(t, path)
+	if err := s.Put("keeper", []byte("must-survive"), nil); err != nil {
+		t.Fatalf("Put: %v", err)
+	}
+	if err := s.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+
+	// Reproduce the wreckage the old swap left behind.
+	if err := os.Rename(path, path+".compact"); err != nil {
+		t.Fatalf("stage orphaned compaction: %v", err)
+	}
+
+	s2 := testStore(t, path)
+	defer s2.Close()
+
+	got, err := s2.Get("keeper")
+	if err != nil {
+		t.Fatalf("data must be recovered from the orphaned .compact file: %v", err)
+	}
+	if string(got) != "must-survive" {
+		t.Fatalf("got %q, want %q", got, "must-survive")
+	}
+	if _, err := os.Stat(path + ".compact"); !os.IsNotExist(err) {
+		t.Fatalf("orphaned compaction file should have been adopted, not copied")
+	}
+}