fix: shutdown admission, recovery identity/order, rollback journal retention, pickup merge

3 files changed, 138 insertions, 2 deletions

diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go
index 7d86cc8..93d186a 100644
--- a/internal/persistence/txn.go
+++ b/internal/persistence/txn.go
@@ -11,6 +11,11 @@ package persistence
 // Transactions do NOT serialize concurrent callers: like single-file saves,
 // they must be executed by an owner that prevents concurrent mutations of
 // the same files (the game's owner goroutine).
+//
+// On apply failure the applied operations are rolled back and the journal is
+// removed. If the ROLLBACK itself fails, the journal is deliberately kept:
+// its before-images are then the only durable record of the pre-transaction
+// state, and the next Recover retries the rollback.
 
 import (
 	"crypto/rand"
@@ -184,7 +189,13 @@ func (t *Transaction) Commit() error {
 
 	if err := t.apply(); err != nil {
 		if rbErr := t.rollback(); rbErr != nil {
-			err = fmt.Errorf("%v; rollback failed: %v", err, rbErr)
+			// The rollback could not restore the before-images: the journal
+			// is now the only durable record of the pre-transaction state.
+			// KEEP it so the next Recover call (or operator repair of the
+			// underlying fault) can retry the rollback instead of silently
+			// losing the before-images forever. See
+			// TestCommitKeepsJournalAfterFailedRollback.
+			return fmt.Errorf("%v; rollback failed (journal kept for recovery: %s): %v", err, journalPath, rbErr)
 		}
 		if rErr := os.Remove(journalPath); rErr != nil && !os.IsNotExist(rErr) {
 			err = fmt.Errorf("%v; journal cleanup failed: %v", err, rErr)
diff --git a/internal/persistence/txn_rollback_test.go b/internal/persistence/txn_rollback_test.go
new file mode 100644
index 0000000..2bd7974
--- /dev/null
+++ b/internal/persistence/txn_rollback_test.go
@@ -0,0 +1,102 @@
+package persistence
+
+// Regression: when a transaction's apply fails AND its rollback fails, the
+// journal is the only durable record of the pre-transaction before-images.
+// Deleting it at commit time made the corruption permanent; it must be kept
+// so the next Recover call (or operator repair) can retry the rollback.
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestCommitKeepsJournalAfterFailedRollback(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "character.yaml")
+	if err := os.WriteFile(path, []byte("before"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	journalDir := filepath.Join(dir, "journals")
+	txn := NewTransaction(journalDir)
+	if err := txn.Replace(path, []byte("after"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	// Deterministically force both the apply and the rollback writes to
+	// fail after the snapshot: the target becomes a nonempty directory,
+	// so WriteFile cannot create the file and rollback cannot restore it.
+	if err := os.Remove(path); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Mkdir(path, 0700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(path, "blocker"), []byte("x"), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	err := txn.Commit()
+	if err == nil || !strings.Contains(err.Error(), "rollback failed") {
+		t.Fatalf("need an apply+rollback failure, got: %v", err)
+	}
+	entries, err := ListJournalFiles(journalDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(entries) == 0 {
+		t.Fatal("failed rollback deleted the only durable before-image; restart cannot recover the pre-transaction state")
+	}
+}
+
+// TestRecoverRetriesKeptRollbackJournal: after the operator repairs the
+// fault that broke the rollback, the next Recover completes it.
+func TestRecoverRetriesKeptRollbackJournal(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "character.yaml")
+	if err := os.WriteFile(path, []byte("before"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	journalDir := filepath.Join(dir, "journals")
+	txn := NewTransaction(journalDir)
+	if err := txn.Replace(path, []byte("after"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(path); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Mkdir(path, 0700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(path, "blocker"), []byte("x"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := txn.Commit(); err == nil {
+		t.Fatal("expected the injected apply/rollback failure")
+	}
+
+	// Operator repair: put the pre-transaction file back.
+	if err := os.RemoveAll(path); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, []byte("after"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := Recover(journalDir); err != nil {
+		t.Fatalf("recovery retry failed: %v", err)
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(data) != "before" {
+		t.Fatalf("recovery did not roll back to the before-image: %q", data)
+	}
+	entries, err := ListJournalFiles(journalDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(entries) != 0 {
+		t.Fatalf("completed recovery left journals behind: %v", entries)
+	}
+}
diff --git a/internal/persistence/txn_test.go b/internal/persistence/txn_test.go
index 10a809d..796cd56 100644
--- a/internal/persistence/txn_test.go
+++ b/internal/persistence/txn_test.go
@@ -99,8 +99,31 @@ func TestTxnRollbackOnFailure(t *testing.T) {
 	if got := readTestFile(t, filepath.Join(sub, "gone.yaml")); got != "old-gone" {
 		t.Fatalf("rollback did not restore deleted file: %q", got)
 	}
+	// The restore of gone.yaml ran inside the still read-only directory, so
+	// the ROLLBACK itself failed there: the journal is the only durable
+	// record of the pre-transaction state and must be KEPT (deleting it
+	// would make any remaining divergence unrecoverable). See
+	// TestCommitKeepsJournalAfterFailedRollback.
+	if entries, _ := ListJournalFiles(journalDir); len(entries) != 1 {
+		t.Fatalf("journal must be kept after a failed rollback: %v", entries)
+	}
+
+	// Repair the directory permission: the next Recover completes the
+	// rollback and cleans the journal.
+	if err := os.Chmod(sub, 0700); err != nil {
+		t.Fatal(err)
+	}
+	if err := Recover(journalDir); err != nil {
+		t.Fatalf("recovery retry after repair: %v", err)
+	}
+	if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "old-a" {
+		t.Fatalf("recovery retry disturbed a.yaml: %q", got)
+	}
+	if got := readTestFile(t, filepath.Join(sub, "gone.yaml")); got != "old-gone" {
+		t.Fatalf("recovery retry disturbed gone.yaml: %q", got)
+	}
 	if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 {
-		t.Fatalf("journal not cleaned after rollback: %v", entries)
+		t.Fatalf("completed recovery left journals behind: %v", entries)
 	}
 }