feat: add insert/remove room to context menu in admin web GUI to 'push' or 'pull' map rooms

1 files changed, 535 insertions, 0 deletions

diff --git a/internal/admin/api_room_insert_remove_test.go b/internal/admin/api_room_insert_remove_test.go
new file mode 100644
index 0000000..4f04a99
--- /dev/null
+++ b/internal/admin/api_room_insert_remove_test.go
@@ -0,0 +1,535 @@
+package admin
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+
+	"thehouseoficarus/internal/world"
+)
+
+// newTestAdminServer builds an AdminServer wired to a temp data dir containing
+// the given room bodies (map of id -> YAML body), with a live World, MobStore,
+// and UndoStack. Only the fields the insert/remove handlers touch are set.
+func newTestAdminServer(t *testing.T, rooms map[int]string) *AdminServer {
+	t.Helper()
+	dir := t.TempDir()
+	roomsDir := filepath.Join(dir, "rooms")
+	if err := os.MkdirAll(roomsDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	for id, body := range rooms {
+		if err := os.WriteFile(filepath.Join(roomsDir, strconv.Itoa(id)+".yaml"), []byte(body), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	return &AdminServer{
+		world:    world.New(dir),
+		mobStore: world.NewMobStore(dir),
+		dataDir:  dir,
+		undoStack: NewUndoStack(dir),
+	}
+}
+
+func writeRoomFile(t *testing.T, dir string, id int, body string) {
+	t.Helper()
+	roomsDir := filepath.Join(dir, "rooms")
+	if err := os.MkdirAll(roomsDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(roomsDir, strconv.Itoa(id)+".yaml"), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func roomExists(s *AdminServer, id int) bool {
+	_, err := s.world.LoadRoom(id)
+	return err == nil
+}
+
+func roomExitTarget(s *AdminServer, id int, dir world.ExitDir) (int, bool) {
+	r, err := s.world.LoadRoom(id)
+	if err != nil {
+		return 0, false
+	}
+	e, ok := r.Exits[dir]
+	if !ok {
+		return 0, false
+	}
+	return e.Room, true
+}
+
+// postInsert drives the insert endpoint with the given body and returns the
+// decoded response alongside the response status code.
+func postInsert(t *testing.T, s *AdminServer, body string) (map[string]any, int) {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/api/rooms/insert", strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	rr := httptest.NewRecorder()
+	s.handleRoomInsert(rr, req)
+	var resp map[string]any
+	_ = json.Unmarshal(rr.Body.Bytes(), &resp)
+	return resp, rr.Code
+}
+
+func postRemove(t *testing.T, s *AdminServer, body string) (map[string]any, int) {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/api/rooms/remove", strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	rr := httptest.NewRecorder()
+	s.handleRoomRemove(rr, req)
+	var resp map[string]any
+	_ = json.Unmarshal(rr.Body.Bytes(), &resp)
+	return resp, rr.Code
+}
+
+// TestInsertSuccess verifies a clean two-way insert creates the new room,
+// rewires A and B onto it, and pushes an undo entry that restores everything.
+func TestInsertSuccess(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  west: 1\n",
+	})
+
+	resp, code := postInsert(t, s, `{"from":1,"dir":"east","name":"Mid"}`)
+	if code != http.StatusOK {
+		t.Fatalf("insert: expected 200, got %d: %v", code, resp)
+	}
+	room := resp["room"].(map[string]any)
+	newID := int(room["id"].(float64))
+	if room["name"] != "Mid" {
+		t.Errorf("insert: expected name Mid, got %v", room["name"])
+	}
+
+	// A→east→new, new→east→2, new→west→1, B→west→new.
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != newID {
+		t.Errorf("A east: expected %d, got %d", newID, tgt)
+	}
+	if tgt, _ := roomExitTarget(s, newID, world.East); tgt != 2 {
+		t.Errorf("new east: expected 2, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, newID, world.West); tgt != 1 {
+		t.Errorf("new west: expected 1, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 2, world.West); tgt != newID {
+		t.Errorf("B west: expected %d, got %d", newID, tgt)
+	}
+
+	// Undo restores the original two-room world.
+	change := s.undoStack.Undo()
+	if change == nil {
+		t.Fatal("undo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(change)
+	if roomExists(s, newID) {
+		t.Error("undo: new room file should be gone")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 2 {
+		t.Errorf("undo: A east should be 2, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 2, world.West); tgt != 1 {
+		t.Errorf("undo: B west should be 1, got %d", tgt)
+	}
+
+	// Redo re-applies the insert.
+	redoChange := s.undoStack.Redo()
+	if redoChange == nil {
+		t.Fatal("redo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(redoChange)
+	if !roomExists(s, newID) {
+		t.Error("redo: new room file should be back")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != newID {
+		t.Errorf("redo: A east should be %d, got %d", newID, tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 2, world.West); tgt != newID {
+		t.Errorf("redo: B west should be %d, got %d", newID, tgt)
+	}
+}
+
+// TestInsertDefaultName confirms an empty name falls back to "New Room".
+func TestInsertDefaultName(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  west: 1\n",
+	})
+	resp, code := postInsert(t, s, `{"from":1,"dir":"east","name":""}`)
+	if code != http.StatusOK {
+		t.Fatalf("insert: expected 200, got %d: %v", code, resp)
+	}
+	if resp["room"].(map[string]any)["name"] != "New Room" {
+		t.Errorf("insert: expected default name 'New Room', got %v", resp["room"].(map[string]any)["name"])
+	}
+}
+
+// TestInsertGridConflict verifies a conflicting insert returns 409 with a
+// human-readable message (not raw JSON, not the old generic string).
+func TestInsertGridConflict(t *testing.T) {
+	// 1→east→2→east→3 and 1→south→4→east→5→east→6→east→7→north→8 at (3,0,0).
+	// Inserting between 1 and 2 pushes 3 to (3,0,0), colliding with 8.
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n  south: 4\n",
+		2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+		3: "name: C\nexits:\n  west: 2\n",
+		4: "name: D\nexits:\n  east: 5\n  north: 1\n",
+		5: "name: E\nexits:\n  east: 6\n  west: 4\n",
+		6: "name: F\nexits:\n  east: 7\n  west: 5\n",
+		7: "name: G\nexits:\n  north: 8\n  west: 6\n",
+		8: "name: H\nexits:\n  south: 7\n",
+	})
+	resp, code := postInsert(t, s, `{"from":1,"dir":"east","name":"X"}`)
+	if code != http.StatusConflict {
+		t.Fatalf("insert conflict: expected 409, got %d: %v", code, resp)
+	}
+	msg, _ := resp["error"].(string)
+	if !strings.Contains(msg, "cannot insert:") {
+		t.Errorf("insert conflict: expected 'cannot insert:' prefix, got %q", msg)
+	}
+	if !strings.Contains(msg, "collision") {
+		t.Errorf("insert conflict: expected overlap wording, got %q", msg)
+	}
+	if !strings.Contains(msg, "#3") || !strings.Contains(msg, "#8") {
+		t.Errorf("insert conflict: expected both colliding room ids (#3 and #8), got %q", msg)
+	}
+	// No partial writes: the world is unchanged.
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 2 {
+		t.Errorf("insert conflict: A east should still be 2, got %d", tgt)
+	}
+}
+
+// TestRemoveSuccess verifies a clean remove deletes B, pulls C back to A, and
+// undo/redo round-trip.
+func TestRemoveSuccess(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+		3: "name: C\nexits:\n  west: 2\n",
+	})
+	resp, code := postRemove(t, s, `{"from":1,"dir":"east"}`)
+	if code != http.StatusOK {
+		t.Fatalf("remove: expected 200, got %d: %v", code, resp)
+	}
+	if int(resp["removed"].(float64)) != 2 {
+		t.Errorf("remove: expected removed=2, got %v", resp["removed"])
+	}
+	if int(resp["pulled_to"].(float64)) != 3 {
+		t.Errorf("remove: expected pulled_to=3, got %v", resp["pulled_to"])
+	}
+	if roomExists(s, 2) {
+		t.Error("remove: B file should be gone")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 3 {
+		t.Errorf("remove: A east should be 3, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 3, world.West); tgt != 1 {
+		t.Errorf("remove: C west should be 1, got %d", tgt)
+	}
+
+	// Undo restores B and reverts A and C.
+	change := s.undoStack.Undo()
+	if change == nil {
+		t.Fatal("undo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(change)
+	if !roomExists(s, 2) {
+		t.Error("undo: B file should be back")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 2 {
+		t.Errorf("undo: A east should be 2, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 3, world.West); tgt != 2 {
+		t.Errorf("undo: C west should be 2, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 2, world.West); tgt != 1 {
+		t.Errorf("undo: B west should be 1, got %d", tgt)
+	}
+	if tgt, _ := roomExitTarget(s, 2, world.East); tgt != 3 {
+		t.Errorf("undo: B east should be 3, got %d", tgt)
+	}
+
+	// Redo re-deletes B and repulls C.
+	redoChange := s.undoStack.Redo()
+	if redoChange == nil {
+		t.Fatal("redo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(redoChange)
+	if roomExists(s, 2) {
+		t.Error("redo: B file should be gone again")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 3 {
+		t.Errorf("redo: A east should be 3, got %d", tgt)
+	}
+}
+
+// TestRemoveGuards exercises each structural guard, checking the 400 status and
+// a human-readable, specific error message.
+func TestRemoveGuards(t *testing.T) {
+	tests := []struct {
+		name  string
+		rooms map[int]string
+		body  string
+		want  string
+	}{
+		{
+			name: "no such exit",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 2\n",
+				2: "name: B\nexits:\n  west: 1\n",
+			},
+			body: `{"from":1,"dir":"north"}`,
+			want: "has no north exit",
+		},
+		{
+			name: "self loop",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 1\n",
+			},
+			body: `{"from":1,"dir":"east"}`,
+			want: "loops back",
+		},
+		{
+			name: "not an insert chain (B does not lead back)",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 2\n",
+				2: "name: B\nexits:\n  east: 3\n",
+				3: "name: C\nexits:\n  west: 2\n",
+			},
+			body: `{"from":1,"dir":"east"}`,
+			want: "not an insert chain",
+		},
+		{
+			name: "dead end (B has no forward exit)",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 2\n",
+				2: "name: B\nexits:\n  west: 1\n",
+			},
+			body: `{"from":1,"dir":"east"}`,
+			want: "no room beyond",
+		},
+		{
+			name: "B has extra exits",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 2\n",
+				2: "name: B\nexits:\n  east: 3\n  west: 1\n  north: 4\n",
+				3: "name: C\nexits:\n  west: 2\n",
+				4: "name: D\nexits:\n  south: 2\n",
+			},
+			body: `{"from":1,"dir":"east"}`,
+			want: "other exits besides",
+		},
+		{
+			name: "C does not lead back to B",
+			rooms: map[int]string{
+				1: "name: A\nexits:\n  east: 2\n",
+				2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+				3: "name: C\nexits:\n  east: 4\n",
+				4: "name: D\nexits:\n  west: 3\n",
+			},
+			body: `{"from":1,"dir":"east"}`,
+			want: "does not lead back to",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			s := newTestAdminServer(t, tc.rooms)
+			resp, code := postRemove(t, s, tc.body)
+			if code != http.StatusBadRequest {
+				t.Fatalf("expected 400, got %d: %v", code, resp)
+			}
+			msg, _ := resp["error"].(string)
+			if !strings.Contains(msg, tc.want) {
+				t.Errorf("expected message containing %q, got %q", tc.want, msg)
+			}
+			if strings.HasPrefix(msg, "{") || strings.Contains(msg, `"error"`) {
+				t.Errorf("error message looks like raw JSON: %q", msg)
+			}
+		})
+	}
+}
+
+// TestRemoveGridConflict verifies a conflicting remove returns 409 with rich
+// wording (overlap) instead of the old generic message.
+func TestRemoveGridConflict(t *testing.T) {
+	// 1→east→2→east→3 (3 has south→6), 1→south→4→east→5. Removing 2 pulls 3
+	// to (1,0,0); 6 (at 3's old south neighbor (2,1,0)) follows to (1,1,0)? No
+	// — the pull shifts the whole beyond-component; 6 lands on 5's cell (1,1,0).
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n  south: 4\n",
+		2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+		3: "name: C\nexits:\n  south: 6\n  west: 2\n",
+		4: "name: D\nexits:\n  east: 5\n  north: 1\n",
+		5: "name: E\nexits:\n  west: 4\n",
+		6: "name: F\nexits:\n  north: 3\n",
+	})
+	resp, code := postRemove(t, s, `{"from":1,"dir":"east"}`)
+	if code != http.StatusConflict {
+		t.Fatalf("remove conflict: expected 409, got %d: %v", code, resp)
+	}
+	msg, _ := resp["error"].(string)
+	if !strings.Contains(msg, "cannot remove:") {
+		t.Errorf("remove conflict: expected 'cannot remove:' prefix, got %q", msg)
+	}
+	if !strings.Contains(msg, "collision") {
+		t.Errorf("remove conflict: expected overlap wording, got %q", msg)
+	}
+	// No partial writes: B is still there, A and C unchanged.
+	if !roomExists(s, 2) {
+		t.Error("remove conflict: B file should still exist")
+	}
+	if tgt, _ := roomExitTarget(s, 1, world.East); tgt != 2 {
+		t.Errorf("remove conflict: A east should still be 2, got %d", tgt)
+	}
+}
+
+// TestRemoveExtraInbound verifies the extra-inbound guard fires when a third
+// room points into B, and that the message names the count.
+func TestRemoveExtraInbound(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+		3: "name: C\nexits:\n  west: 2\n",
+		9: "name: X\nexits:\n  north: 2\n", // extra inbound edge into B
+	})
+	resp, code := postRemove(t, s, `{"from":1,"dir":"east"}`)
+	if code != http.StatusBadRequest {
+		t.Fatalf("expected 400, got %d: %v", code, resp)
+	}
+	msg, _ := resp["error"].(string)
+	if !strings.Contains(msg, "other exit(s) point into") {
+		t.Errorf("expected extra-inbound message, got %q", msg)
+	}
+}
+
+// TestNextRoomIDNoCrossSubdirCollision confirms the allocator does not return an
+// ID that already exists in a different subdirectory.
+func TestNextRoomIDNoCrossSubdirCollision(t *testing.T) {
+	dir := t.TempDir()
+	roomsDir := filepath.Join(dir, "rooms")
+	if err := os.MkdirAll(roomsDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	// Room 1 in root, room 2 in a subdirectory "zone".
+	writeRoomFile(t, dir, 1, "name: A\nexits:\n  east: 2\n")
+	zoneDir := filepath.Join(roomsDir, "zone")
+	if err := os.MkdirAll(zoneDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(zoneDir, "2.yaml"), []byte("name: B\nexits:\n  west: 1\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	s := &AdminServer{
+		world:    world.New(dir),
+		mobStore: world.NewMobStore(dir),
+		dataDir:  dir,
+		undoStack: NewUndoStack(dir),
+	}
+	// Allocating near room 1 (root) must skip 2 (which lives in zone/).
+	id, _, err := s.nextRoomID(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if id == 2 {
+		t.Fatalf("nextRoomID returned %d, which collides with the zone/ room", id)
+	}
+	if roomExists(s, id) {
+		t.Errorf("nextRoomID returned an already-used id %d", id)
+	}
+}
+
+// testMobDef returns a minimal mob def with enough HP that MobsInRoom (which
+// filters HP > 0) reports it as present.
+func testMobDef() *world.MobDef {
+	return &world.MobDef{
+		ID:   "testmob",
+		Name: "Test",
+		Combat: &world.MobCombat{
+			Stats: world.MobCombatStats{HP: 10, MaxMeleeHit: 1},
+		},
+	}
+}
+
+// TestUndoInsertClearsMobs verifies that undoing an insert drops in-memory mob
+// instances for the (now-deleted) new room, via rebuildAfterUndo's stat check.
+func TestUndoInsertClearsMobs(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  west: 1\n",
+	})
+	resp, code := postInsert(t, s, `{"from":1,"dir":"east","name":"Mid"}`)
+	if code != http.StatusOK {
+		t.Fatalf("insert: expected 200, got %d: %v", code, resp)
+	}
+	newID := int(resp["room"].(map[string]any)["id"].(float64))
+
+	// Spawn a transient mob in the new room so the store has something to clear.
+	inst := s.mobStore.SpawnTransient(testMobDef(), &world.SpawnMobConfig{ID: "testmob"}, newID, "")
+	if inst == nil {
+		t.Fatal("SpawnTransient returned nil")
+	}
+	if got := len(s.mobStore.MobsInRoom(newID)); got != 1 {
+		t.Fatalf("precondition: expected 1 mob in new room, got %d", got)
+	}
+
+	change := s.undoStack.Undo()
+	if change == nil {
+		t.Fatal("undo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(change)
+
+	if got := len(s.mobStore.MobsInRoom(newID)); got != 0 {
+		t.Errorf("undo: expected mobs for deleted room to be cleared, got %d", got)
+	}
+}
+
+// TestRedoRemoveClearsMobs verifies that redoing a remove drops mob instances
+// for the re-deleted room.
+func TestRedoRemoveClearsMobs(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{
+		1: "name: A\nexits:\n  east: 2\n",
+		2: "name: B\nexits:\n  east: 3\n  west: 1\n",
+		3: "name: C\nexits:\n  west: 2\n",
+	})
+	if _, code := postRemove(t, s, `{"from":1,"dir":"east"}`); code != http.StatusOK {
+		t.Fatal("remove failed")
+	}
+	// Undo to bring B back, then seed a mob in B before redoing.
+	change := s.undoStack.Undo()
+	if change == nil {
+		t.Fatal("undo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(change)
+	if !roomExists(s, 2) {
+		t.Fatal("precondition: B should exist after undo")
+	}
+	s.mobStore.SpawnTransient(testMobDef(), &world.SpawnMobConfig{ID: "testmob"}, 2, "")
+	if got := len(s.mobStore.MobsInRoom(2)); got != 1 {
+		t.Fatalf("precondition: expected 1 mob in B, got %d", got)
+	}
+
+	redoChange := s.undoStack.Redo()
+	if redoChange == nil {
+		t.Fatal("redo: expected a change, got nil")
+	}
+	s.rebuildAfterUndo(redoChange)
+
+	if got := len(s.mobStore.MobsInRoom(2)); got != 0 {
+		t.Errorf("redo: expected mobs for re-deleted B to be cleared, got %d", got)
+	}
+}
+
+// TestInsertInvalidBody confirms malformed input yields 400, not a panic.
+func TestInsertInvalidBody(t *testing.T) {
+	s := newTestAdminServer(t, map[int]string{1: "name: A\n"})
+	for _, body := range []string{`{}`, `{"from":0,"dir":"east"}`, `{"from":1,"dir":""}`, `{"from":1,"dir":"sideways"}`} {
+		resp, code := postInsert(t, s, body)
+		if code != http.StatusBadRequest {
+			t.Errorf("insert %q: expected 400, got %d: %v", body, code, resp)
+		}
+	}
+}