4 files changed, 164 insertions, 16 deletions

diff --git a/internal/admin/api_entity_path_test.go b/internal/admin/api_entity_path_test.go
index 7abf65f..27ecbe8 100644
--- a/internal/admin/api_entity_path_test.go
+++ b/internal/admin/api_entity_path_test.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"thehouseoficarus/internal/behavior"
 	"thehouseoficarus/internal/config"
 	"thehouseoficarus/internal/player"
 )
@@ -60,3 +61,35 @@ func TestEntityAPICannotEscapeData(t *testing.T) {
 		t.Fatal("entity API escaped the data directory")
 	}
 }
+
+// TestEntityRenameRejectsDuplicateIDInOtherDirectory: entity IDs are global
+// across the type's subdirectory tree (the runtime path indexes resolve by
+// filename anywhere under data/<subdir>), so renaming into an ID that
+// exists in a DIFFERENT directory must be rejected instead of creating a
+// duplicate ID shadowed by whichever file the index finds first.
+func TestEntityRenameRejectsDuplicateIDInOtherDirectory(t *testing.T) {
+	dir := t.TempDir()
+	for _, sub := range []string{"items/a", "items/b"} {
+		if err := os.MkdirAll(filepath.Join(dir, sub), 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.WriteFile(filepath.Join(dir, "items/a/old.yaml"), []byte("name: Original\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "items/b/taken.yaml"), []byte("name: Existing\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, _, _, err := renameEntityFile(dir, "items", "old", "taken"); err == nil {
+		t.Fatal("rename accepted an ID that already exists in another directory")
+	}
+	if dups := behavior.CheckDuplicateIDs(filepath.Join(dir, "items")); len(dups) != 0 {
+		t.Fatalf("rename produced duplicate IDs: %+v", dups)
+	}
+
+	// Renaming within the same directory to a free name still works.
+	if _, _, _, err := renameEntityFile(dir, "items", "old", "moved"); err != nil {
+		t.Fatalf("ordinary rename rejected: %v", err)
+	}
+}
diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go
index 16fc7a0..f9508f6 100644
--- a/internal/admin/api_rooms.go
+++ b/internal/admin/api_rooms.go
@@ -144,11 +144,38 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) {
 
 		var extraFiles []ExtraFile
 
+		// Prepare the reciprocal exit in the destination map BEFORE the
+		// destination file is committed, so the written file is complete.
+		if linkFrom != nil && linkDir != nil && *linkDir != "" {
+			if dir := world.ExitDir(strings.ToLower(*linkDir)); dir != "" {
+				if opp, ok := world.OppositeExit[dir]; ok && !linkOneWay {
+					exits[string(opp)] = map[string]any{"room": *linkFrom}
+				}
+			}
+		}
+
+		// The destination room is committed FIRST: if it cannot be written,
+		// nothing else has been mutated yet. The old order wrote the
+		// source room's new exit first, so a failed destination write left
+		// a dangling exit pointing at a room that never existed, with no
+		// undo entry to repair it.
+		path := filepath.Join(subdir, strconv.Itoa(id)+".yaml")
+		newContent, writeErr := writeMapAsYAML(path, m)
+		if writeErr != nil {
+			failJSON(w, fmt.Sprintf("write room: %v", writeErr))
+			return
+		}
+
 		if linkFrom != nil && linkDir != nil && *linkDir != "" {
 			dir := world.ExitDir(strings.ToLower(*linkDir))
-			if opp, ok := world.OppositeExit[dir]; ok {
+			if _, ok := world.OppositeExit[dir]; ok {
 				srcRoom, loadErr := s.world.LoadRoom(*linkFrom)
 				if loadErr != nil {
+					// Roll back the destination so a failed create leaves
+					// no half-created room behind.
+					if rmErr := os.Remove(path); rmErr != nil {
+						log.Printf("room create rollback: remove %s: %v", path, rmErr)
+					}
 					failJSON(w, fmt.Sprintf("load link_from room %d: %v", *linkFrom, loadErr))
 					return
 				}
@@ -160,31 +187,23 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) {
 				srcPath, srcOk := s.world.GetRoomPath(*linkFrom)
 				if srcOk {
 					oldContent, _ := snapshotFile(srcPath)
-					newContent, writeErr := writeYAMLFile(srcPath, srcRoom)
+					srcContent, writeErr := writeYAMLFile(srcPath, srcRoom)
 					if writeErr != nil {
+						if rmErr := os.Remove(path); rmErr != nil {
+							log.Printf("room create rollback: remove %s: %v", path, rmErr)
+						}
 						failJSON(w, fmt.Sprintf("write source room: %v", writeErr))
 						return
 					}
 					extraFiles = append(extraFiles, ExtraFile{
 						FilePath:   srcPath,
 						OldContent: oldContent,
-						NewContent: newContent,
+						NewContent: srcContent,
 					})
 				}
-
-				if !linkOneWay {
-					exits[string(opp)] = map[string]any{"room": *linkFrom}
-				}
 			}
 		}
 
-		path := filepath.Join(subdir, strconv.Itoa(id)+".yaml")
-		newContent, writeErr := writeMapAsYAML(path, m)
-		if writeErr != nil {
-			failJSON(w, fmt.Sprintf("write room: %v", writeErr))
-			return
-		}
-
 		s.world.AddRoomPath(id, path)
 		s.undoStack.Push(ChangeDesc{
 			Description: fmt.Sprintf("create room %d", id),
diff --git a/internal/admin/api_rooms_create_test.go b/internal/admin/api_rooms_create_test.go
new file mode 100644
index 0000000..38d6fae
--- /dev/null
+++ b/internal/admin/api_rooms_create_test.go
@@ -0,0 +1,78 @@
+package admin
+
+// Regression: a failed room create (POST /api/rooms with link_from) must
+// not leave the source room with a dangling exit pointing at a room that
+// was never written, and must not push an undo entry for work that never
+// committed. The destination is committed before the source exit so the
+// failure happens before any source-side mutation.
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"thehouseoficarus/internal/config"
+	"thehouseoficarus/internal/world"
+)
+
+func TestFailedRoomCreateRollsBackSourceExit(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{1: "id: 1\nname: Origin\n"})
+	s.cfg = config.Default()
+	s.cfg.StartingRoom = 1
+	// A directory at the destination path makes the destination file write
+	// fail deterministically while the source room is still readable.
+	if err := os.Mkdir(filepath.Join(s.dataDir, "rooms", "2.yaml"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	rec := httptest.NewRecorder()
+	s.handleRooms(rec, httptest.NewRequest(http.MethodPost, "/api/rooms",
+		strings.NewReader(`{"name":"Next","link_from":1,"link_dir":"north"}`)))
+	if !strings.Contains(rec.Body.String(), "error") {
+		t.Fatalf("expected the create to fail; got %s", rec.Body.String())
+	}
+
+	src, err := s.world.LoadRoom(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if exit, ok := src.Exits[world.North]; ok {
+		t.Fatalf("failed create left a dangling exit to nonexistent room %d", exit.Room)
+	}
+	if info := s.undoStack.Info(); info.StackSize != 0 {
+		t.Fatalf("failed create pushed an undo entry: %+v", info)
+	}
+}
+
+// The reciprocal exit still lands in the written destination file when the
+// create succeeds.
+func TestSuccessfulRoomCreateWritesReciprocalExit(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{1: "id: 1\nname: Origin\n"})
+	s.cfg = config.Default()
+	s.cfg.StartingRoom = 1
+
+	rec := httptest.NewRecorder()
+	s.handleRooms(rec, httptest.NewRequest(http.MethodPost, "/api/rooms",
+		strings.NewReader(`{"name":"Next","link_from":1,"link_dir":"north"}`)))
+	if strings.Contains(rec.Body.String(), "error") {
+		t.Fatalf("create failed: %s", rec.Body.String())
+	}
+
+	dest, err := s.world.LoadRoom(2)
+	if err != nil {
+		t.Fatalf("destination room not loadable: %v", err)
+	}
+	if dest.Exits[world.South].Room != 1 {
+		t.Fatalf("destination room missing its reciprocal exit south: %+v", dest.Exits)
+	}
+	src, err := s.world.LoadRoom(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if src.Exits[world.North].Room != 2 {
+		t.Fatalf("source room missing its north exit: %+v", src.Exits)
+	}
+}
diff --git a/internal/admin/yaml_util.go b/internal/admin/yaml_util.go
index 6ea6692..c042248 100644
--- a/internal/admin/yaml_util.go
+++ b/internal/admin/yaml_util.go
@@ -226,8 +226,26 @@ func renameEntityFile(dataDir, subdir, oldID, newID string) (string, string, []b
 	if oldPath == newPath {
 		return "", "", nil, fmt.Errorf("new name is same as current")
 	}
-	if _, err := os.Stat(newPath); err == nil {
-		return "", "", nil, fmt.Errorf("entity %s already exists", newID)
+	// The ID namespace is global across the entity type's subdirectory tree
+	// (findEntityPath and the runtime path indexes resolve by filename
+	// anywhere under data/<subdir>), so the collision check must cover the
+	// whole tree, not just the file's own directory — otherwise renaming
+	// into an ID that exists in ANOTHER directory creates a duplicate ID
+	// shadowed by whichever file the index happens to find first.
+	target := newID + ".yaml"
+	var clash string
+	filepath.WalkDir(filepath.Join(dataDir, subdir), func(path string, d os.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return nil
+		}
+		if d.Name() == target {
+			clash = path
+			return filepath.SkipAll
+		}
+		return nil
+	})
+	if clash != "" {
+		return "", "", nil, fmt.Errorf("entity %s already exists at %s", newID, clash)
 	}
 	data, err := os.ReadFile(oldPath)
 	if err != nil {