2 files changed, 93 insertions, 0 deletions
diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go
index 400d355..1049077 100644
--- a/internal/persistence/txn.go
+++ b/internal/persistence/txn.go
@@ -179,6 +179,9 @@ func (t *Transaction) Replace(path string, data []byte, perm os.FileMode) error
 	if err != nil {
 		return err
 	}
+	if op.Kind == kindMove {
+		return fmt.Errorf("conflicting operation on %s: already moved to %s", path, op.MoveTo)
+	}
 	if op.Kind == kindMoveTarget {
 		op.New = data
 		op.NewExists = true
@@ -198,6 +201,9 @@ func (t *Transaction) Delete(path string) error {
 	if err != nil {
 		return err
 	}
+	if op.Kind == kindMove {
+		return fmt.Errorf("conflicting operation on %s: already moved to %s", path, op.MoveTo)
+	}
 	op.Kind = kindDelete
 	op.New, op.NewExists = nil, false
 	return nil
@@ -217,6 +223,9 @@ func (t *Transaction) Move(oldPath, newPath string) error {
 	if src.Kind == kindMove && src.MoveTo != newPath {
 		return fmt.Errorf("conflicting move of %s", oldPath)
 	}
+	if src.Kind == kindReplace || src.Kind == kindDelete {
+		return fmt.Errorf("conflicting operation on %s: already replaced or deleted", oldPath)
+	}
 	if dst.Kind != kindMoveTarget && dst.Kind != "" {
 		return fmt.Errorf("conflicting operation on %s", newPath)
 	}
@@ -370,6 +379,12 @@ func (t *Transaction) rollback() error {
 			if err := restore(op.Path); err != nil {
 				record(fmt.Errorf("restore %s: %w", op.Path, err))
 			}
+		default:
+			// An unknown kind (a journal written by a newer version, or a
+			// hand-edited file) must not be silently skipped: the before-image
+			// would be dropped and the journal then deleted, losing the only
+			// record of the pre-transaction state. Fail so Recover keeps it.
+			record(fmt.Errorf("unknown op kind %q for %s", op.Kind, op.Path))
 		}
 	}
 	return firstErr
@@ -381,6 +396,11 @@ func Recover(journalDir string) error {
 	if err := os.MkdirAll(journalDir, 0700); err != nil {
 		return err
 	}
+	// Same durability requirement as Commit: persist the directory entry so a
+	// crash cannot erase the journals before they are recovered.
+	if err := syncDir(filepath.Dir(journalDir)); err != nil {
+		return fmt.Errorf("sync journal parent: %w", err)
+	}
 	entries, err := os.ReadDir(journalDir)
 	if err != nil {
 		return err
diff --git a/internal/persistence/txn_test.go b/internal/persistence/txn_test.go
index 796cd56..be19c7c 100644
--- a/internal/persistence/txn_test.go
+++ b/internal/persistence/txn_test.go
@@ -207,3 +207,76 @@ func TestTxnMoveOverwritesDestination(t *testing.T) {
 		t.Fatal("unreachable")
 	}
 }
+
+// Regression: Move followed by Delete/Replace on the same path used to clobber
+// the move record, silently losing the file with a nil Commit error.
+func TestTxnRejectsConflictingMoveThenDelete(t *testing.T) {
+	dir := t.TempDir()
+	src := filepath.Join(dir, "src.yaml")
+	dst := filepath.Join(dir, "dst.yaml")
+	writeTestFile(t, src, "content")
+	txn := NewTransaction(filepath.Join(dir, ".journal"))
+	if err := txn.Move(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	if err := txn.Delete(src); err == nil {
+		t.Fatal("Delete after Move on the same path was accepted")
+	}
+}
+
+func TestTxnRejectsConflictingMoveThenReplace(t *testing.T) {
+	dir := t.TempDir()
+	src := filepath.Join(dir, "src.yaml")
+	dst := filepath.Join(dir, "dst.yaml")
+	writeTestFile(t, src, "content")
+	txn := NewTransaction(filepath.Join(dir, ".journal"))
+	if err := txn.Move(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	if err := txn.Replace(src, []byte("new"), 0644); err == nil {
+		t.Fatal("Replace after Move on the same path was accepted")
+	}
+}
+
+// Move then Replace on the destination is the supported pattern and must still
+// work.
+func TestTxnMoveThenReplaceDestination(t *testing.T) {
+	dir := t.TempDir()
+	src := filepath.Join(dir, "src.yaml")
+	dst := filepath.Join(dir, "dst.yaml")
+	writeTestFile(t, src, "content")
+	txn := NewTransaction(filepath.Join(dir, ".journal"))
+	if err := txn.Move(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	if err := txn.Replace(dst, []byte("rewritten"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := txn.Commit(); err != nil {
+		t.Fatal(err)
+	}
+	if got := readTestFile(t, dst); got != "rewritten" {
+		t.Fatalf("dst = %q, want rewritten", got)
+	}
+}
+
+// Regression: an unrecognised op kind must abort recovery and keep the journal
+// rather than silently dropping the before-image and deleting the record.
+func TestRecoverKeepsJournalWithUnknownOpKind(t *testing.T) {
+	dir := t.TempDir()
+	journalDir := filepath.Join(dir, ".journal")
+	if err := os.MkdirAll(journalDir, 0700); err != nil {
+		t.Fatal(err)
+	}
+	journal := []byte(`{"ops":[{"path":"` + filepath.Join(dir, "a.yaml") + `","kind":"future-kind","old_exists":false}]}`)
+	path := filepath.Join(journalDir, "txn-unknown.json")
+	if err := os.WriteFile(path, journal, 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := Recover(journalDir); err == nil {
+		t.Fatal("Recover accepted an unknown op kind")
+	}
+	if _, err := os.Stat(path); err != nil {
+		t.Fatalf("journal with unknown kind was removed: %v", err)
+	}
+}