fix: admin undo/redo failure, video poker cabinet ids, room ref behavior

5 files changed, 73 insertions, 1 deletions

diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go
index 586bfff..76db231 100644
--- a/internal/admin/api_rooms.go
+++ b/internal/admin/api_rooms.go
@@ -1315,6 +1315,25 @@ func (s *AdminServer) handleResolveDuplicate(w http.ResponseWriter, r *http.Requ
 		failJSON(w, "invalid path")
 		return
 	}
+	// Only a real duplicate-ID file may be resolved here. Trusting the client
+	// path would let this endpoint delete a live indexed room with none of the
+	// normal delete semantics (exit cleanup, casino eviction, undo).
+	stem := strings.TrimSuffix(filepath.Base(absPath), ".yaml")
+	isDuplicate := false
+	for _, d := range behavior.CheckDuplicateIDs(roomsDir) {
+		if d.ID != stem {
+			continue
+		}
+		for _, p := range d.Paths {
+			if pa, err := filepath.Abs(p); err == nil && pa == absPath {
+				isDuplicate = true
+			}
+		}
+	}
+	if !isDuplicate {
+		failJSON(w, "path is not a duplicate-ID room file")
+		return
+	}
 
 	switch body.Action {
 	case "delete":
diff --git a/internal/admin/api_triggers.go b/internal/admin/api_triggers.go
index 0dd086c..ff3e7d6 100644
--- a/internal/admin/api_triggers.go
+++ b/internal/admin/api_triggers.go
@@ -56,8 +56,12 @@ func (s *AdminServer) handleTriggers(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		newContent, err := writeMapAsYAML(path, m)
+		newContent, err := triggerMapToYAML(m)
 		if err != nil {
+			failJSON(w, "serialize error: "+err.Error())
+			return
+		}
+		if err := persistence.WriteFile(path, newContent, 0644); err != nil {
 			failJSON(w, "write error: "+err.Error())
 			return
 		}
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index 60073f7..d97732d 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -49,3 +49,27 @@ func TestRestoreAccountSecretRefusesNonMappingBody(t *testing.T) {
 		t.Fatalf("password hash not preserved: %s", out)
 	}
 }
+
+// Regression: a failed undo returned a nil change exactly like an empty
+// history, so the UI reported "Nothing to undo" for a real failure.
+func TestUndoFailureIsReportedDistinctly(t *testing.T) {
+	dir := t.TempDir()
+	us := NewUndoStack(dir)
+	us.Push(ChangeDesc{
+		Description: "edit with a missing before-image",
+		FilePath:    filepath.Join(dir, "rooms", "1.yaml"),
+		OldContent:  nil,
+		NewContent:  []byte("id: 1\n"),
+	})
+	if ch := us.Undo(); ch != nil {
+		t.Fatal("undo with a missing before-image unexpectedly succeeded")
+	}
+	if err := us.LastError(); err == nil {
+		t.Fatal("LastError = nil after a failed undo; the UI would report 'Nothing to undo'")
+	}
+	// A genuinely empty history is a clean nil change and nil error.
+	us2 := NewUndoStack(t.TempDir())
+	if ch := us2.Undo(); ch != nil || us2.LastError() != nil {
+		t.Fatal("empty history must be a clean nil change and nil error")
+	}
+}
diff --git a/internal/admin/server.go b/internal/admin/server.go
index 8fcba77..800cfa1 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -908,6 +908,10 @@ func (s *AdminServer) doUndo(w http.ResponseWriter, r *http.Request) {
 	}
 	change := s.undoStack.Undo()
 	if change == nil {
+		if err := s.undoStack.LastError(); err != nil {
+			writeJSONError(w, "undo failed: "+err.Error(), http.StatusInternalServerError)
+			return
+		}
 		writeJSON(w, map[string]any{"message": "Nothing to undo"})
 		return
 	}
@@ -927,6 +931,10 @@ func (s *AdminServer) doRedo(w http.ResponseWriter, r *http.Request) {
 	}
 	change := s.undoStack.Redo()
 	if change == nil {
+		if err := s.undoStack.LastError(); err != nil {
+			writeJSONError(w, "redo failed: "+err.Error(), http.StatusInternalServerError)
+			return
+		}
 		writeJSON(w, map[string]any{"message": "Nothing to redo"})
 		return
 	}
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index 5683f13..c3231e3 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -85,6 +85,10 @@ type UndoStack struct {
 	// writes those files directly, so it must refuse the same files the raw
 	// file editor refuses. Optional; nil disables the refusal.
 	playerFileOnlineCheck func(relPath string) bool
+	// lastErr records the most recent Undo/Redo failure so the HTTP handler
+	// can distinguish a genuine failure from an empty history (both return a
+	// nil change). Owner-task access only.
+	lastErr error
 }
 
 func NewUndoStack(dataDir string) *UndoStack {
@@ -179,8 +183,10 @@ func (us *UndoStack) Undo() *ChangeDesc {
 
 	if err := us.applyUndo(last); err != nil {
 		log.Printf("undo: failed %s: %v", last.ShortDesc(), err)
+		us.lastErr = err
 		return nil
 	}
+	us.lastErr = nil
 	// Runtime migration moves with the file images: undo applies the
 	// inverse room-ID map so players, Hub membership, and casino room keys
 	// return to the old location together.
@@ -209,8 +215,10 @@ func (us *UndoStack) Redo() *ChangeDesc {
 
 	if err := us.applyRedo(last); err != nil {
 		log.Printf("redo: failed %s: %v", last.ShortDesc(), err)
+		us.lastErr = err
 		return nil
 	}
+	us.lastErr = nil
 	// Redo applies the forward room-ID map through the runtime migration.
 	if len(last.IDMap) > 0 && us.migrateRoomIDs != nil {
 		forward := make(map[int]int, len(last.IDMap))
@@ -803,6 +811,15 @@ func (us *UndoStack) Clear() {
 	defer us.mu.Unlock()
 	us.history = nil
 	us.redo = nil
+	us.lastErr = nil
 	os.Remove(us.filePath)
 	log.Printf("undo: cleared history")
 }
+
+// LastError returns the error from the most recent failed Undo/Redo, or nil.
+// A nil change plus a nil LastError means the history/redo stack was empty.
+func (us *UndoStack) LastError() error {
+	us.mu.Lock()
+	defer us.mu.Unlock()
+	return us.lastErr
+}