From 6f3c43aca62a7d33a19f14f6ad7c16891952673a Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Tue, 15 Sep 2026 19:55:21 -0400 Subject: fix: outstanding-stake ledger, owner backpressure, editor state preservation --- internal/admin/api_rooms.go | 12 +-- internal/admin/api_rooms_bulk.go | 146 +++++++++++++--------------- internal/admin/bulk_edit_regression_test.go | 125 ++++++++++++++++++++++++ internal/admin/room_rename_state_test.go | 68 +++++++++++++ internal/admin/server.go | 29 ++++-- 5 files changed, 287 insertions(+), 93 deletions(-) create mode 100644 internal/admin/bulk_edit_regression_test.go create mode 100644 internal/admin/room_rename_state_test.go (limited to 'internal/admin') diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index 4cfb467..c93e7c6 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -970,13 +970,13 @@ func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { } s.world.RebuildRoomIndex(s.dataDir) - s.world.ClearRoomState(body.ID) - // Migrate room-ID-embedded state across all characters (discovered-exit - // player flags, RoomID, EnterSeqRoom, MapSymbols, RoomsVisited) plus - // Hub membership buckets, casino room keys, world room-keyed state, and - // live mob instances, so a rename leaves no gameplay, broadcast, or - // player attached to the old location. + // Runtime room state moves with the room: ground items, seeded spawn + // markers, and object states are rekeyed to the new ID — never cleared. + // Clearing here would destroy live room contents. The wired game + // callback performs the same world rekey plus players, Hub membership, + // casino room keys, and mob instances. + s.world.RekeyRooms(idMap) if s.rewriteRoomIDs != nil { s.rewriteRoomIDs(idMap) } diff --git a/internal/admin/api_rooms_bulk.go b/internal/admin/api_rooms_bulk.go index 6283067..a7fcee7 100644 --- a/internal/admin/api_rooms_bulk.go +++ b/internal/admin/api_rooms_bulk.go @@ -8,6 +8,8 @@ import ( "os" "path/filepath" "strconv" + + "thehouseoficarus/internal/persistence" ) type bulkEditPatch struct { @@ -22,6 +24,17 @@ type bulkEditRequest struct { IDs []int `json:"ids"` } +// handleBulkEdit applies one patch to many rooms as a single mutation +// plan: +// +// 1. Preflight reads, parses, patches, and marshals every selected room in +// memory — including destination-directory and conflict checks for +// moves. Any failure aborts BEFORE any file is touched: a failed bulk +// edit never leaves partial changes behind. +// 2. The writes apply as one journal transaction (in-place replaces, and +// move+replace for real directory moves) that rolls back on failure. +// 3. One undo record captures every before/after image, so undo restores +// the whole plan. func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) @@ -53,43 +66,27 @@ func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { patch := req.Patch - roomPaths := make([]string, 0, len(req.IDs)) - for _, id := range req.IDs { - path, ok := s.world.GetRoomPath(id) - if !ok { - writeJSON(w, map[string]any{"error": fmt.Sprintf("room %d not found", id)}) - return - } - roomPaths = append(roomPaths, path) - } - type roomWrite struct { - oldContent []byte - newContent []byte - oldPath string - newPath string - wasMoved bool + id int + oldPath, newPath string + oldContent, newContent []byte } + // ---- Preflight: everything that can fail happens here, before any + // file is modified. ---- writes := make([]roomWrite, 0, len(req.IDs)) - dirMovePaths := make(map[string]struct{}) - - for i, id := range req.IDs { - path := roomPaths[i] - - oldContent, snapErr := snapshotFile(path) - if snapErr != nil { - writeJSON(w, map[string]any{"error": fmt.Sprintf("snapshot room %d: %v", id, snapErr)}) + for _, id := range req.IDs { + path, ok := s.world.GetRoomPath(id) + if !ok { + writeJSON(w, map[string]any{"error": fmt.Sprintf("room %d not found", id)}) return } - - data, readErr := os.ReadFile(path) - if readErr != nil { - writeJSON(w, map[string]any{"error": fmt.Sprintf("read room %d: %v", id, readErr)}) + oldContent, err := os.ReadFile(path) + if err != nil { + writeJSON(w, map[string]any{"error": fmt.Sprintf("read room %d: %v", id, err)}) return } - - m, yamlErr := yamlToMap(data) + m, yamlErr := yamlToMap(oldContent) if yamlErr != nil { writeJSON(w, map[string]any{"error": fmt.Sprintf("parse room %d: %v", id, yamlErr)}) return @@ -109,63 +106,50 @@ func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { } } + newPath := path if patch.Dir != nil && *patch.Dir != "" { destDir := filepath.Join(s.dataDir, "rooms", *patch.Dir) - if _, err := os.Stat(destDir); os.IsNotExist(err) { + if _, statErr := os.Stat(destDir); os.IsNotExist(statErr) { writeJSON(w, map[string]any{"error": fmt.Sprintf("directory does not exist: %s", *patch.Dir)}) return } - - fileName := strconv.Itoa(id) + ".yaml" - newPath := filepath.Join(destDir, fileName) + newPath = filepath.Join(destDir, strconv.Itoa(id)+".yaml") if newPath != path { - if _, err := os.Stat(newPath); err == nil { + if _, statErr := os.Stat(newPath); statErr == nil { writeJSON(w, map[string]any{"error": fmt.Sprintf("room %d already exists in target directory", id)}) return } } + // newPath == path is an in-place edit: the room already lives + // in the target directory and must NOT be removed after its + // write (that deleted existing rooms). + } - newContent, writeErr := writeMapAsYAML(newPath, m) - if writeErr != nil { - writeJSON(w, map[string]any{"error": fmt.Sprintf("write room %d: %v", id, writeErr)}) - return - } - - writes = append(writes, roomWrite{ - oldContent: oldContent, - newContent: newContent, - oldPath: path, - newPath: newPath, - wasMoved: true, - }) - dirMovePaths[path] = struct{}{} - } else { - newContent, writeErr := writeMapAsYAML(path, m) - if writeErr != nil { - writeJSON(w, map[string]any{"error": fmt.Sprintf("write room %d: %v", id, writeErr)}) - return - } - - writes = append(writes, roomWrite{ - oldContent: oldContent, - newContent: newContent, - oldPath: path, - newPath: path, - wasMoved: false, - }) + newContent, marshalErr := marshalMapYAML(m) + if marshalErr != nil { + writeJSON(w, map[string]any{"error": fmt.Sprintf("marshal room %d: %v", id, marshalErr)}) + return } + writes = append(writes, roomWrite{id: id, oldPath: path, newPath: newPath, oldContent: oldContent, newContent: newContent}) } - for _, rw := range writes { - if rw.wasMoved { - if _, alreadyRemoved := dirMovePaths[rw.oldPath]; alreadyRemoved { - delete(dirMovePaths, rw.oldPath) - if err := os.Remove(rw.oldPath); err != nil { - writeJSON(w, map[string]any{"error": fmt.Sprintf("remove old room: %v", err)}) - return - } + // ---- Apply as one journaled transaction. ---- + txn := persistence.NewTransaction(filepath.Join(s.dataDir, ".journal")) + for _, wr := range writes { + if wr.newPath != wr.oldPath { + if err := txn.Move(wr.oldPath, wr.newPath); err != nil { + writeJSON(w, map[string]any{"error": fmt.Sprintf("move room %d: %v", wr.id, err)}) + return } } + if err := txn.Replace(wr.newPath, wr.newContent, 0644); err != nil { + writeJSON(w, map[string]any{"error": fmt.Sprintf("write room %d: %v", wr.id, err)}) + return + } + } + if err := txn.Commit(); err != nil { + writeJSON(w, map[string]any{"error": fmt.Sprintf("commit bulk edit: %v", err)}) + return } s.world.RebuildRoomIndex(s.dataDir) @@ -179,24 +163,24 @@ func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { var extraFiles []ExtraFile var firstDesc ChangeDesc - for i, rw := range writes { + for i, wr := range writes { ef := ExtraFile{ - FilePath: rw.oldPath, - OldContent: rw.oldContent, - NewContent: rw.newContent, + FilePath: wr.oldPath, + OldContent: wr.oldContent, + NewContent: wr.newContent, } - if rw.wasMoved { - ef.NewFilePath = rw.newPath + if wr.newPath != wr.oldPath { + ef.NewFilePath = wr.newPath } if i == 0 { firstDesc = ChangeDesc{ Description: desc, - FilePath: rw.oldPath, - OldContent: rw.oldContent, - NewContent: rw.newContent, + FilePath: wr.oldPath, + OldContent: wr.oldContent, + NewContent: wr.newContent, } - if rw.wasMoved { - firstDesc.NewFilePath = rw.newPath + if wr.newPath != wr.oldPath { + firstDesc.NewFilePath = wr.newPath } } else { extraFiles = append(extraFiles, ef) diff --git a/internal/admin/bulk_edit_regression_test.go b/internal/admin/bulk_edit_regression_test.go new file mode 100644 index 0000000..4828c93 --- /dev/null +++ b/internal/admin/bulk_edit_regression_test.go @@ -0,0 +1,125 @@ +package admin + +// Bulk-edit mutation-plan regressions: a same-directory "move" must never +// delete the room, a failure anywhere must leave every room untouched (no +// partial changes without an undo record), and a successful plan applies as +// one unit with a single undo record. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func postBulkEdit(t *testing.T, s *AdminServer, body string) (map[string]any, int) { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/rooms/bulk-edit", strings.NewReader(body)) + rec := httptest.NewRecorder() + s.handleBulkEdit(rec, req) + var resp map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &resp) + return resp, rec.Code +} + +// TestBulkEditSameDirectoryMovePreservesRoom: moving a room to the +// directory it already lives in is an in-place edit, not a delete. The old +// implementation removed the file and reported success. +func TestBulkEditSameDirectoryMovePreservesRoom(t *testing.T) { + s := newTestAdminServer(t, map[int]string{1: "id: 1\nname: Room\n"}) + oldPath, _ := s.world.GetRoomPath(1) + dir := filepath.Join(s.dataDir, "rooms", "area") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + dest := filepath.Join(dir, "1.yaml") + if err := os.Rename(oldPath, dest); err != nil { + t.Fatal(err) + } + s.world.RebuildRoomIndex(s.dataDir) + + resp, code := postBulkEdit(t, s, `{"ids":[1],"patch":{"dir":"area"}}`) + if code != http.StatusOK { + t.Fatalf("bulk edit failed: %d %v", code, resp) + } + if _, err := os.Stat(dest); err != nil { + t.Fatalf("same-directory move deleted the room: %v", err) + } + if _, err := os.Stat(oldPath); !os.IsNotExist(err) { + t.Fatal("old path should not reappear for an in-place edit") + } +} + +// TestBulkEditFailureLeavesNoPartialChanges: a parse failure on any selected +// room aborts the whole plan before any file changes, so no room is mutated +// without an undo record. +func TestBulkEditFailureLeavesNoPartialChanges(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 1: "id: 1\nname: Room\n", + 2: "invalid: [\n", + }) + path, _ := s.world.GetRoomPath(1) + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + resp, code := postBulkEdit(t, s, `{"ids":[1,2],"patch":{"color":"red"}}`) + if code == http.StatusOK && resp["error"] == nil { + t.Fatalf("bulk edit unexpectedly succeeded: %v", resp) + } + if resp["error"] == nil { + t.Fatalf("expected an error response, got: %v", resp) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Fatalf("failed bulk edit mutated room 1: before=%q after=%q", before, after) + } + if s.undoStack.Info().StackSize != 0 { + t.Fatal("failed bulk edit pushed an undo record") + } +} + +// TestBulkEditSuccessSingleUndoRecord: a valid plan applies every patch and +// pushes exactly one undo record whose undo restores every file. +func TestBulkEditSuccessSingleUndoRecord(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 1: "id: 1\nname: Room\n", + 2: "id: 2\nname: Other\n", + }) + + resp, code := postBulkEdit(t, s, `{"ids":[1,2],"patch":{"color":"red"}}`) + if code != http.StatusOK { + t.Fatalf("bulk edit failed: %d %v", code, resp) + } + for _, id := range []int{1, 2} { + path, _ := s.world.GetRoomPath(id) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "color: red") { + t.Fatalf("room %d not patched: %q", id, data) + } + } + if info := s.undoStack.Info(); info.StackSize != 1 { + t.Fatalf("undo stack size = %d, want exactly 1 plan", info.StackSize) + } + if s.undoStack.Undo() == nil { + t.Fatal("undo failed") + } + path, _ := s.world.GetRoomPath(1) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "color: red") { + t.Fatalf("undo did not restore room 1: %q", data) + } +} diff --git a/internal/admin/room_rename_state_test.go b/internal/admin/room_rename_state_test.go new file mode 100644 index 0000000..8c5be93 --- /dev/null +++ b/internal/admin/room_rename_state_test.go @@ -0,0 +1,68 @@ +package admin + +// Room-rename runtime-state regressions: renaming a room must move its live +// contents (ground items, seeded spawn markers, object state) to the new ID +// — never clear them — and undo must bring them back. + +import ( + "testing" +) + +// TestRoomRenameMovesRuntimeState: renaming a room with ground items moves +// them to the new room ID. The old implementation cleared the state before +// the migration callback could rekey it, destroying the room's contents. +func TestRoomRenameMovesRuntimeState(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 1: "id: 1\nname: Room\nexits:\n east:\n room: 2\n", + 2: "id: 2\nname: Neighbor\nexits:\n west:\n room: 1\n", + }) + s.world.AddGroundItem(1, "chips", 50) + s.world.AddGroundItem(1, "chips", 10) // second entry: rekey must move the whole list + s.rewriteRoomIDs = func(idMap map[int]int) { s.world.RekeyRooms(idMap) } + s.undoStack.SetMigrateRoomIDs(s.rewriteRoomIDs) + + if rec := renameRoom(t, s, 1, 3); rec.Code != 200 { + t.Fatalf("rename failed: %d %s", rec.Code, rec.Body.String()) + } + if got := s.world.GroundItems(3)["chips"]; got != 60 { + t.Fatalf("ground items after rename = %d, want 60 in room 3", got) + } + if got := s.world.GroundItems(1)["chips"]; got != 0 { + t.Fatalf("ground items left behind in the old room: %d", got) + } + + // Undo restores the files AND the runtime state through the inverse map. + if s.undoStack.Undo() == nil { + t.Fatal("undo failed") + } + if got := s.world.GroundItems(1)["chips"]; got != 60 { + t.Fatalf("ground items after undo = %d, want 60 back in room 1", got) + } +} + +// TestRoomRenameUndoDoesNotClearSurvivingRoomState: undoing a rename must +// not clear runtime state of rooms whose files still exist afterwards +// (only undo-of-create / redo-of-delete drops state). +func TestRoomRenameUndoDoesNotClearSurvivingRoomState(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 1: "id: 1\nname: Room\n", + 2: "id: 2\nname: Unrelated\n", + }) + s.world.AddGroundItem(2, "chips", 5) + s.rewriteRoomIDs = func(idMap map[int]int) { s.world.RekeyRooms(idMap) } + s.undoStack.SetMigrateRoomIDs(s.rewriteRoomIDs) + + if rec := renameRoom(t, s, 1, 3); rec.Code != 200 { + t.Fatalf("rename failed: %d %s", rec.Code, rec.Body.String()) + } + // The unrelated room's state must survive the rename and its undo. + if got := s.world.GroundItems(2)["chips"]; got != 5 { + t.Fatalf("unrelated room state lost during rename: %d", got) + } + if s.undoStack.Undo() == nil { + t.Fatal("undo failed") + } + if got := s.world.GroundItems(2)["chips"]; got != 5 { + t.Fatalf("unrelated room state lost during undo: %d", got) + } +} diff --git a/internal/admin/server.go b/internal/admin/server.go index 41c1e0f..81f0cce 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -364,6 +364,17 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) { return } + // Transparent upgrade: legacy single-pass SHA-256 hashes rehash to + // Argon2id after a successful verification. + if player.NeedsRehash(acc.PasswordHash) { + if hash, hashErr := player.HashPassword(password); hashErr == nil { + acc.PasswordHash = hash + if err := s.accountStore.SaveAccount(acc); err != nil { + log.Printf("admin login: password rehash save for %s: %v", account, err) + } + } + } + if !acc.Admin { s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."}) return @@ -649,12 +660,18 @@ func (s *AdminServer) rebuildAfterUndo(change *ChangeDesc) { name := filepath.Base(p) if idStr := strings.TrimSuffix(name, ".yaml"); idStr != name { if id, err := strconv.Atoi(idStr); err == nil && id > 0 { - s.world.ClearRoomState(id) - // If the room file is now gone (true after undo-of-create and - // redo-of-delete), drop any in-memory mob instances for it so - // they don't keep ticking against a room that no longer exists. - if _, statErr := os.Stat(p); os.IsNotExist(statErr) && s.mobStore != nil { - s.mobStore.RemoveMobsInRoom(id) + // Only drop runtime room state when the room file is now gone + // (undo-of-create, redo-of-delete, directory deletion): a + // rename's undo restores the file and the inverse migration + // rekeys its runtime state back — clearing it here would + // destroy the room's live contents. + if _, statErr := os.Stat(p); os.IsNotExist(statErr) { + s.world.ClearRoomState(id) + // Drop any in-memory mob instances too so they don't keep + // ticking against a room that no longer exists. + if s.mobStore != nil { + s.mobStore.RemoveMobsInRoom(id) + } } } } -- cgit v1.2.3