fix: registration overwrite, recovery chip/payout edge cases, bulk-delete rollback

2 files changed, 92 insertions, 0 deletions

diff --git a/internal/admin/api_rooms_bulk_delete.go b/internal/admin/api_rooms_bulk_delete.go
index 18b10a5..70c2eb6 100644
--- a/internal/admin/api_rooms_bulk_delete.go
+++ b/internal/admin/api_rooms_bulk_delete.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"log"
 	"net/http"
 	"os"
 
@@ -38,6 +39,18 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Duplicate IDs in one request would delete the same file twice: the
+	// second os.Remove fails after the room is already gone, and the
+	// handler aborts without pushing an undo record. Reject up front.
+	seenIDs := make(map[int]bool, len(req.IDs))
+	for _, id := range req.IDs {
+		if seenIDs[id] {
+			writeJSON(w, map[string]any{"error": fmt.Sprintf("duplicate room id %d in request", id)})
+			return
+		}
+		seenIDs[id] = true
+	}
+
 	type roomSnapshot struct {
 		id         int
 		path       string
@@ -63,6 +76,15 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) {
 	for _, snap := range snapshots {
 		s.detachRoomFromCourse(snap.id)
 		if err := os.Remove(snap.path); err != nil {
+			// All-or-nothing: restore every room deleted so far so a failed
+			// bulk delete never strands rooms without an undo record.
+			for _, done := range snapshots {
+				if deletedSet[done.id] {
+					if werr := os.WriteFile(done.path, done.oldContent, 0644); werr != nil {
+						log.Printf("bulk-delete rollback of room %d: %v", done.id, werr)
+					}
+				}
+			}
 			writeJSON(w, map[string]any{"error": fmt.Sprintf("delete room %d: %v", snap.id, err)})
 			return
 		}
diff --git a/internal/admin/api_rooms_bulk_delete_test.go b/internal/admin/api_rooms_bulk_delete_test.go
new file mode 100644
index 0000000..083b890
--- /dev/null
+++ b/internal/admin/api_rooms_bulk_delete_test.go
@@ -0,0 +1,70 @@
+package admin
+
+// Bulk-delete regression tests: duplicate IDs in one request are rejected
+// before any file is touched, and a mid-plan delete failure rolls back
+// every room it already removed (no rooms stranded without an undo record).
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func postBulkDelete(t *testing.T, s *AdminServer, body string) (map[string]any, int) {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/api/rooms/bulk-delete", strings.NewReader(body))
+	rec := httptest.NewRecorder()
+	s.handleBulkDelete(rec, req)
+	var resp map[string]any
+	_ = json.Unmarshal(rec.Body.Bytes(), &resp)
+	return resp, rec.Code
+}
+
+func TestBulkDeleteRejectsDuplicateIDs(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{1: "id: 1\nname: Room\n"})
+	path, _ := s.world.GetRoomPath(1)
+	resp, _ := postBulkDelete(t, s, `{"ids":[1,1]}`)
+	if errMsg, _ := resp["error"].(string); !strings.Contains(errMsg, "duplicate room id 1") {
+		t.Fatalf("duplicate IDs not rejected: %v", resp)
+	}
+	if _, err := os.Stat(path); err != nil {
+		t.Fatalf("rejected bulk-delete removed the room: %v", err)
+	}
+}
+
+// TestBulkDeleteFailureRollsBackDeletions: a delete failure mid-plan
+// restores the rooms already removed, so a failed plan leaves the world
+// untouched. The failure is staged at the DELETE phase (not the earlier
+// snapshot phase) by making room 3's parent directory unwritable after its
+// file snapshot was taken.
+func TestBulkDeleteFailureRollsBackDeletions(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{1: "id: 1\nname: One\n"})
+	sub := filepath.Join(s.dataDir, "rooms", "sub")
+	if err := os.MkdirAll(sub, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(sub, "3.yaml"), []byte("id: 3\nname: Three\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	s.world.RebuildRoomIndex(s.dataDir)
+
+	// Room 3's snapshot succeeds (the file is readable), but its removal
+	// fails because the parent directory is not writable.
+	if err := os.Chmod(sub, 0o555); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(sub, 0o755); os.RemoveAll(sub) })
+
+	path1, _ := s.world.GetRoomPath(1)
+	resp, _ := postBulkDelete(t, s, `{"ids":[1,3]}`)
+	if errMsg, _ := resp["error"].(string); !strings.Contains(errMsg, "delete room 3") {
+		t.Fatalf("expected room-3 delete failure: %v", resp)
+	}
+	if _, err := os.Stat(path1); err != nil {
+		t.Fatalf("failed bulk delete stranded room 1 without an undo record: %v", err)
+	}
+}