aboutsummaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/game/cmd_get.go11
-rw-r--r--internal/game/economy.go61
-rw-r--r--internal/game/economy_recovery_identity_test.go128
-rw-r--r--internal/game/game.go28
-rw-r--r--internal/game/hacking/liars_dice.go10
-rw-r--r--internal/game/hacking/liars_dice_test.go68
-rw-r--r--internal/game/inventory_pickup_test.go55
-rw-r--r--internal/game/owner.go9
-rw-r--r--internal/game/owner_regression_test.go14
-rw-r--r--internal/game/shutdown_admission_test.go55
-rw-r--r--internal/persistence/txn.go13
-rw-r--r--internal/persistence/txn_rollback_test.go102
-rw-r--r--internal/persistence/txn_test.go25
13 files changed, 538 insertions, 41 deletions
diff --git a/internal/game/cmd_get.go b/internal/game/cmd_get.go
index d538afc..515d2ab 100644
--- a/internal/game/cmd_get.go
+++ b/internal/game/cmd_get.go
@@ -68,11 +68,10 @@ func (g *Game) doGet(sess *net.Session, input string) {
qty = available
}
- freeSlot := p.FirstFreeSlot()
- if freeSlot == -1 {
- sess.WriteLine("Your inventory is full.")
- return
- }
+ // No premature free-slot gate here: a stackable pickup may merge into an
+ // existing stack even when every slot is occupied, so capacity is only
+ // required when a new slot is actually needed (matching doGetAll and
+ // doGetAllNamed). See TestPickupMergesIntoFullInventory.
if def != nil && def.Stackable {
for i := 0; i < 28; i++ {
@@ -89,7 +88,7 @@ func (g *Game) doGet(sess *net.Session, input string) {
return
}
}
- freeSlot = p.FirstFreeSlot()
+ freeSlot := p.FirstFreeSlot()
if freeSlot == -1 {
sess.WriteLine("Your inventory is full.")
return
diff --git a/internal/game/economy.go b/internal/game/economy.go
index e917fa6..be3f5ef 100644
--- a/internal/game/economy.go
+++ b/internal/game/economy.go
@@ -234,6 +234,24 @@ func (rec *economyRecord) key() string {
return rec.Player
}
+// payoutIdentifies reports whether two journal records belong to the same
+// character identity. ID-carrying records match only on the stable ID: a
+// different character that reused the display name must never supply (or
+// consume) another character's resolution evidence. Legacy records without
+// IDs keep the historical name-based match among themselves.
+func payoutIdentifies(a, b economyRecord) bool {
+ if a.PlayerID != "" && b.PlayerID != "" {
+ return a.PlayerID == b.PlayerID
+ }
+ if a.PlayerID != "" || b.PlayerID != "" {
+ // Mixed legacy/ID records: refuse to match on name alone so an ID
+ // can never be resolved through a name that another character may
+ // have reused.
+ return false
+ }
+ return strings.EqualFold(a.Player, b.Player)
+}
+
// reloadStakes rebuilds the outstanding-stake index from the journal file:
// open wager records remain tracked (and refundable/resolvable) after
// recovery compaction.
@@ -435,16 +453,14 @@ func (g *Game) recoverEconomy() {
}
loadPlayer := func(rec *economyRecord) *player.Player {
if rec.PlayerID != "" {
+ // A record carrying a stable ID resolves to THAT identity only.
+ // Never fall back to the display name: a different character
+ // that later reused the name would collect the original
+ // character's payout and the original liability would be
+ // settled as paid. See TestRecoveryDoesNotPayReusedCharacterName.
if p, ok := playersByID[rec.PlayerID]; ok {
return p
}
- }
- if rec.Player != "" {
- if p, ok := playersByName[strings.ToLower(rec.Player)]; ok {
- return p
- }
- }
- if rec.PlayerID != "" {
// A character file added since the index was built: load every
// file once and match by stable ID.
if names, err := g.AccountStore.ListCharacterNames(); err == nil {
@@ -456,11 +472,20 @@ func (g *Game) recoverEconomy() {
}
}
}
+ // Identity not found (character deleted or unreadable): leave
+ // the record open so a later attempt (or operator inspection)
+ // can resolve it; do not silently drop or reassign money.
+ fmt.Fprintf(os.Stderr, "economy recovery: load %s (id %q): character not found\n", rec.Player, rec.PlayerID)
+ return nil
+ }
+ // Legacy record written before stable IDs existed: the (renameable)
+ // name is the only identity available.
+ if rec.Player != "" {
+ if p, ok := playersByName[strings.ToLower(rec.Player)]; ok {
+ return p
+ }
}
- // Character file unreadable: leave the record open so a later
- // attempt (or operator inspection) can resolve it; do not
- // silently drop money.
- fmt.Fprintf(os.Stderr, "economy recovery: load %s (id %q): character not found\n", rec.Player, rec.PlayerID)
+ fmt.Fprintf(os.Stderr, "economy recovery: load %s: character not found\n", rec.Player)
return nil
}
// laterPayoutEvidence reports whether any non-void payout record was
@@ -476,9 +501,10 @@ func (g *Game) recoverEconomy() {
// instead marks a refund in progress: its credit part is re-applied by
// phase 1 and the chip part (if the debit was durable) still comes
// back in chips.
- laterPayoutEvidence := func(name string, seq int) (resolved bool, settlement bool) {
+ laterPayoutEvidence := func(rec *economyRecord, seq int) (resolved bool, settlement bool) {
for _, fr := range folded {
- if fr.rec.Kind == "payout" && fr.rec.Player == name && fr.rec.Status != "void" && fr.seq > seq {
+ if fr.rec.Kind == "payout" && fr.rec.Status != "void" && fr.seq > seq &&
+ payoutIdentifies(fr.rec, *rec) {
resolved = true
if !fr.rec.Refund {
settlement = true
@@ -499,6 +525,9 @@ func (g *Game) recoverEconomy() {
// re-application as its own durability and silently drop the owed
// payout — the group is settled together instead.
type payoutSig struct {
+ // player is the record's stable identity key (PlayerID when known,
+ // else the name): grouping by display name alone would merge two
+ // different characters that reused the same name.
player string
before, after int
credit int
@@ -507,7 +536,7 @@ func (g *Game) recoverEconomy() {
for i := range folded {
rec := &folded[i].rec
if rec.Kind == "payout" && rec.Status == "open" {
- sig := payoutSig{rec.Player, rec.CreditsBefore, rec.CreditsAfter, rec.Credit}
+ sig := payoutSig{rec.key(), rec.CreditsBefore, rec.CreditsAfter, rec.Credit}
siblings[sig] = append(siblings[sig], rec)
}
}
@@ -521,7 +550,7 @@ func (g *Game) recoverEconomy() {
if p == nil {
continue
}
- sig := payoutSig{rec.Player, rec.CreditsBefore, rec.CreditsAfter, rec.Credit}
+ sig := payoutSig{rec.key(), rec.CreditsBefore, rec.CreditsAfter, rec.Credit}
group := siblings[sig]
switch {
case p.Credits == rec.CreditsBefore:
@@ -589,7 +618,7 @@ func (g *Game) recoverEconomy() {
}
for k := len(openWagers) - 1; k >= 0; k-- {
rec := &folded[openWagers[k]].rec
- resolvedByPayout, resolvedBySettlement := laterPayoutEvidence(rec.Player, folded[openWagers[k]].seq)
+ resolvedByPayout, resolvedBySettlement := laterPayoutEvidence(rec, folded[openWagers[k]].seq)
p := loadPlayer(rec)
if p == nil {
continue
diff --git a/internal/game/economy_recovery_identity_test.go b/internal/game/economy_recovery_identity_test.go
new file mode 100644
index 0000000..eceeecc
--- /dev/null
+++ b/internal/game/economy_recovery_identity_test.go
@@ -0,0 +1,128 @@
+package game
+
+// Identity-resolution regressions for economy recovery: a record carrying a
+// stable character ID must never be resolved — paid or resolved — through a
+// display name that a different character later reused, and money recovery
+// must run only AFTER lifecycle-journal rollback (a rolled-back rename
+// restores an older character before-image that must not erase compensation
+// already applied).
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "thehouseoficarus/internal/persistence"
+)
+
+// TestRecoveryDoesNotPayReusedCharacterName: an owed payout journaled for
+// identity A (named "alice") must not be paid to identity B after B reuses
+// the name, and A's unresolved liability must stay open for operator
+// inspection instead of being silently settled against B's balance.
+func TestRecoveryDoesNotPayReusedCharacterName(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ p := h.seedCharacter("alice", "alice")
+ h.g.RunOnOwner(func() {
+ p.ID = "original-character"
+ p.Credits = 90
+ if err := h.g.AccountStore.SaveCharacter(p); err != nil {
+ t.Error(err)
+ return
+ }
+ // A failed payout save left the owed money journaled for recovery.
+ if _, err := h.g.Economy.start("payout", p, 0, 20, 90, 110, 0, 0); err != nil {
+ t.Error(err)
+ return
+ }
+ // Delete the original character, then reuse its name for a
+ // different identity.
+ p.ID = "replacement-character"
+ if err := h.g.AccountStore.SaveCharacter(p); err != nil {
+ t.Error(err)
+ return
+ }
+ h.g.recoverEconomy()
+ })
+ loaded, err := h.g.AccountStore.LoadCharacter("alice")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if loaded.Credits != 90 {
+ t.Errorf("replacement identity received someone else's payout: credits=%d, want 90", loaded.Credits)
+ }
+ if open := h.g.Economy.loadOpen(); len(open) != 1 {
+ t.Errorf("original identity's unresolved liability was discarded: open records=%d", len(open))
+ }
+}
+
+// TestEconomyRecoveryRunsAfterLifecycleRollback: a leftover lifecycle
+// transaction (a rename that never reached its commit point) restores the
+// character's before-image at startup, and must do so BEFORE money
+// compensation — otherwise the compensation's settled/compacted records are
+// gone and the rollback silently erases a payout that was already applied.
+func TestEconomyRecoveryRunsAfterLifecycleRollback(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ p := h.seedCharacter("alice", "alice")
+ h.g.RunOnOwner(func() {
+ p.ID = "stable-character"
+ p.Credits = 90
+ if err := h.g.AccountStore.SaveCharacter(p); err != nil {
+ t.Error(err)
+ return
+ }
+ // A failed payout save left the owed money journaled for recovery.
+ if _, err := h.g.Economy.start("payout", p, 0, 20, 90, 110, 0, 0); err != nil {
+ t.Error(err)
+ return
+ }
+ // Simulate a crash mid-rename: apply the move and the renamed
+ // character file directly, then leave the lifecycle journal on
+ // disk so the next startup rolls it back.
+ oldPath := h.g.AccountStore.CharPath("alice")
+ newPath := h.g.AccountStore.CharPath("renamed")
+ txn := persistence.NewTransaction(h.g.journalDir())
+ if err := txn.Move(oldPath, newPath); err != nil {
+ t.Error(err)
+ return
+ }
+ p.Name = "renamed"
+ data, err := h.g.AccountStore.MarshalCharacter(p)
+ if err != nil {
+ t.Error(err)
+ return
+ }
+ if err := txn.Replace(newPath, data, 0600); err != nil {
+ t.Error(err)
+ return
+ }
+ journal, err := txn.MarshalJournal()
+ if err != nil {
+ t.Error(err)
+ return
+ }
+ if err := os.WriteFile(filepath.Join(h.g.journalDir(), "txn-audit.json"), journal, 0600); err != nil {
+ t.Error(err)
+ return
+ }
+ if err := os.Rename(oldPath, newPath); err != nil {
+ t.Error(err)
+ return
+ }
+ if err := h.g.AccountStore.SaveCharacter(p); err != nil {
+ t.Error(err)
+ return
+ }
+ })
+ // A fresh Game over the saved files simulates the restart.
+ restarted := harnessFromDir(t, h.dir)
+ loaded, err := restarted.g.AccountStore.LoadCharacter("alice")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if loaded.Credits != 110 {
+ t.Errorf("lifecycle rollback erased recovered payout: credits=%d, want 110; open liabilities=%d",
+ loaded.Credits, len(restarted.g.Economy.loadOpen()))
+ }
+}
diff --git a/internal/game/game.go b/internal/game/game.go
index 7f29d1f..ca38e27 100644
--- a/internal/game/game.go
+++ b/internal/game/game.go
@@ -135,6 +135,15 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig *
// initialization cycle with the shutdown coordinator.
g.shutdownHook = g.RequestShutdown
g.Economy = newEconomyJournal(dataDir)
+ // Lifecycle-journal rollback MUST run before economy recovery: a rolled-
+ // back rename transaction restores an older character before-image, and
+ // if money compensation ran first its settled/compacted records would no
+ // longer be recoverable — the rollback would silently erase a payout
+ // that was already re-applied. Rollback also precedes every store read
+ // below (courses, mods, techs, craft index) that a leftover transaction
+ // may reference. See the TestEconomyRecoveryRunsAfterLifecycleRollback
+ // regression.
+ g.recoverJournals()
// Compensate open money records from any prior crash before any
// session can connect.
g.recoverEconomy()
@@ -144,9 +153,7 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig *
g.LoadTechs()
g.buildCraftIndex()
g.loadAllFlagTriggers()
- // Journal recovery before any store reads files a leftover transaction
- // may reference, then one-time stable-identity migration.
- g.recoverJournals()
+ // One-time stable-identity migration.
g.backfillCharacterIDs()
g.ValidateAndLog()
return g
@@ -273,9 +280,22 @@ func (g *Game) saveAllOnlineCharacters() error {
// HandleSessionOnOwner is the entry point handed to the network server:
// connection goroutines submit one task per input line, so all game state
-// mutation executes on the owner goroutine.
+// mutation executes on the owner goroutine. Once shutdown has begun, new
+// input is REJECTED (not queued and not executed inline): the shutdown
+// sequence owns final persistence, and the transports keep draining until
+// they close, so an accepted line could mutate characters after the final
+// save or race the refund/save barrier.
func (g *Game) HandleSessionOnOwner(sess *net.Session, input string) {
+ if g.isShuttingDown() {
+ return
+ }
g.RunOnOwner(func() {
+ // Re-checked at task execution: the flag may flip between
+ // submission and execution, and RunOnOwner must never fall back
+ // to inline execution after the owner has stopped.
+ if g.isShuttingDown() {
+ return
+ }
g.HandleSession(sess, input)
})
}
diff --git a/internal/game/hacking/liars_dice.go b/internal/game/hacking/liars_dice.go
index 1dd27b6..8e21b94 100644
--- a/internal/game/hacking/liars_dice.go
+++ b/internal/game/hacking/liars_dice.go
@@ -175,7 +175,7 @@ func (l *LiarsDiceGame) doCall(player bool) (string, bool, bool) {
}
}
- return l.checkGameOver(sb)
+ return l.checkGameOver(&sb)
}
func (l *LiarsDiceGame) doExact() (string, bool, bool) {
@@ -202,10 +202,14 @@ func (l *LiarsDiceGame) doExact() (string, bool, bool) {
l.playerLost++
}
- return l.checkGameOver(sb)
+ return l.checkGameOver(&sb)
}
-func (l *LiarsDiceGame) checkGameOver(sb strings.Builder) (string, bool, bool) {
+// checkGameOver resolves a completed challenge/exact exchange. The builder is
+// passed BY POINTER: it already holds the resolution text, and copying a
+// non-zero strings.Builder by value panics — a panic here runs on the game
+// owner goroutine, which has no recovery, so it would terminate the server.
+func (l *LiarsDiceGame) checkGameOver(sb *strings.Builder) (string, bool, bool) {
if len(l.playerDice) == 0 {
sb.WriteString("\nYou've lost all your fragments. Connection terminated.")
l.gameOver = true
diff --git a/internal/game/hacking/liars_dice_test.go b/internal/game/hacking/liars_dice_test.go
new file mode 100644
index 0000000..59ebb92
--- /dev/null
+++ b/internal/game/hacking/liars_dice_test.go
@@ -0,0 +1,68 @@
+package hacking
+
+import (
+ "strings"
+ "testing"
+)
+
+// Regression: round resolution (challenge and exact calls) passed an
+// already-used strings.Builder BY VALUE into checkGameOver, panicking with
+// "illegal use of non-zero Builder copied by value" on ordinary input — a
+// forced challenge resolves on the very first bid. A panic here runs on the
+// game owner goroutine, which has no recovery, so it terminated the server.
+// See building_guide/concurrency.md for the owner's no-recovery guarantee.
+
+func TestLiarsDiceForcedChallengeDoesNotPanic(t *testing.T) {
+ g := NewLiarsDiceGame()
+ g.Init(1)
+ // An impossible bid deterministically forces an immediate AI challenge.
+ out, _, _ := g.HandleInput("bid 100 6")
+ if !strings.Contains(out, "Actual count") {
+ t.Fatalf("challenge output missing the reveal: %q", out)
+ }
+ if !strings.Contains(out, "You lose a fragment") && !strings.Contains(out, "System loses a fragment") {
+ t.Fatalf("challenge did not resolve a fragment loss: %q", out)
+ }
+}
+
+func TestLiarsDiceCallAndExactResolveWithoutPanic(t *testing.T) {
+ // Call path: bid, let the system raise, then challenge it.
+ g := NewLiarsDiceGame()
+ g.Init(1)
+ if out, _, _ := g.HandleInput("bid 2 3"); out == "" {
+ t.Fatal("opening bid produced no output")
+ }
+ if out, _, _ := g.HandleInput("call"); out == "" {
+ t.Fatal("call produced no output")
+ }
+
+ // Exact path: bid, let the system raise, then claim exact.
+ g = NewLiarsDiceGame()
+ g.Init(1)
+ if out, _, _ := g.HandleInput("bid 2 3"); out == "" {
+ t.Fatal("opening bid produced no output")
+ }
+ if out, _, _ := g.HandleInput("exact"); out == "" {
+ t.Fatal("exact produced no output")
+ }
+}
+
+func TestLiarsDiceFullLossGameResolvesWithoutPanic(t *testing.T) {
+ // Bid high enough that the system always calls and the player always
+ // loses a fragment, driving the game to its terminal resolution.
+ g := NewLiarsDiceGame()
+ g.Init(1)
+ for i := 0; i < 10; i++ {
+ out, done, won := g.HandleInput("bid 100 6")
+ if done {
+ if won {
+ t.Fatal("all-in losing bids cannot win")
+ }
+ return
+ }
+ if out == "" {
+ t.Fatal("round produced no output")
+ }
+ }
+ t.Fatal("game did not terminate within 10 forced-loss rounds")
+}
diff --git a/internal/game/inventory_pickup_test.go b/internal/game/inventory_pickup_test.go
new file mode 100644
index 0000000..a0d267e
--- /dev/null
+++ b/internal/game/inventory_pickup_test.go
@@ -0,0 +1,55 @@
+package game
+
+// Regression: picking up a stackable item must merge into an existing stack
+// even when every inventory slot is occupied — capacity is required only
+// when a NEW slot is needed, matching the get-all paths.
+
+import (
+ "path/filepath"
+ "testing"
+
+ "thehouseoficarus/internal/net"
+ "thehouseoficarus/internal/player"
+)
+
+func TestPickupMergesIntoFullInventory(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ p := h.seedCharacter("alice", "alice")
+ writeSeedFile(t, filepath.Join(h.dir, "items", "chips.yaml"), []byte("id: chips\nname: chips\nstackable: true\n"))
+ sess := &net.Session{Conn: newChanConn(), Player: p, State: net.StateGame}
+ h.g.RunOnOwner(func() {
+ h.g.ItemStore.Reload(h.dir)
+ for i := 0; i < 28; i++ {
+ p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1})
+ }
+ p.SetInvSlot(0, &player.InventorySlot{ItemID: "chips", Quantity: 10})
+ h.g.World.AddGroundItem(1, "chips", 5)
+ h.g.doGet(sess, "chips")
+ if got := countChips(p); got != 15 {
+ t.Errorf("pickup rejected despite existing chip stack: have %d, want 15", got)
+ }
+ })
+}
+
+func TestPickupFullInventoryWithoutStackStillRefused(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ p := h.seedCharacter("alice", "alice")
+ writeSeedFile(t, filepath.Join(h.dir, "items", "chips.yaml"), []byte("id: chips\nname: chips\nstackable: true\n"))
+ sess := &net.Session{Conn: newChanConn(), Player: p, State: net.StateGame}
+ h.g.RunOnOwner(func() {
+ h.g.ItemStore.Reload(h.dir)
+ for i := 0; i < 28; i++ {
+ p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1})
+ }
+ h.g.World.AddGroundItem(1, "chips", 5)
+ h.g.doGet(sess, "chips")
+ if got := countChips(p); got != 0 {
+ t.Errorf("pickup without an existing stack must be refused, have %d", got)
+ }
+ if got := h.g.World.GroundItems(1)["chips"]; got != 5 {
+ t.Errorf("refused pickup must leave the ground stack intact, have %d", got)
+ }
+ })
+}
diff --git a/internal/game/owner.go b/internal/game/owner.go
index 8546d10..ce1f687 100644
--- a/internal/game/owner.go
+++ b/internal/game/owner.go
@@ -136,10 +136,11 @@ func (g *Game) run(fn func()) {
close(task)
}) {
// The owner loop has stopped (graceful shutdown in progress).
- // Admissions are already closed, so only stragglers reach this:
- // execute inline rather than block forever. A full queue never
- // takes this path: enqueue applies backpressure instead.
- fn()
+ // Admissions are closed: DROP the task. Executing it on the
+ // caller's goroutine would mutate live state concurrently with the
+ // shutdown sequence and after the final-save barrier (network
+ // transports are still draining until transportStop runs). See
+ // TestShutdownRejectsSessionInput.
return
}
<-task
diff --git a/internal/game/owner_regression_test.go b/internal/game/owner_regression_test.go
index 1ccd25f..3ba4bd4 100644
--- a/internal/game/owner_regression_test.go
+++ b/internal/game/owner_regression_test.go
@@ -52,17 +52,19 @@ func TestFullOwnerQueueAppliesBackpressure(t *testing.T) {
}
}
-// TestStoppedOwnerRunsStragglersInline: after the owner stops (graceful
-// shutdown), straggler submissions execute inline rather than blocking
-// forever — the documented admissions-closed behavior.
-func TestStoppedOwnerRunsStragglersInline(t *testing.T) {
+// TestStoppedOwnerDropsStragglers: after the owner stops (graceful
+// shutdown), straggler submissions are dropped, never executed inline on the
+// caller's goroutine (which would mutate live state concurrently with the
+// shutdown sequence and after the final-save barrier) and never block
+// forever.
+func TestStoppedOwnerDropsStragglers(t *testing.T) {
g := &Game{owner: newOwner()}
g.stopOwner()
ran := make(chan struct{})
go g.run(func() { close(ran) })
select {
case <-ran:
- case <-time.After(5 * time.Second):
- t.Fatal("straggler task blocked forever after the owner stopped")
+ t.Fatal("straggler task executed after the owner stopped; it must be dropped")
+ case <-time.After(100 * time.Millisecond):
}
}
diff --git a/internal/game/shutdown_admission_test.go b/internal/game/shutdown_admission_test.go
new file mode 100644
index 0000000..6aa4be5
--- /dev/null
+++ b/internal/game/shutdown_admission_test.go
@@ -0,0 +1,55 @@
+package game
+
+// Shutdown-admission regression: once shutdown has begun, session input must
+// never mutate game state — not during the drain window, and especially not
+// after the owner has stopped (the transports keep draining until they
+// close, so an accepted line would execute concurrently with the shutdown
+// sequence or after the final-save barrier).
+
+import (
+ "testing"
+
+ "thehouseoficarus/internal/net"
+)
+
+func TestShutdownRejectsSessionInput(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ h.seedCharacter("alice", "alice")
+ sess, _ := h.connect("alice")
+ sess.State = net.StateGame
+ h.g.setShuttingDown()
+ h.g.stopOwner()
+
+ // The network listeners are closed only AFTER the owner stops, so
+ // connections can still submit lines through the production entry
+ // point. Every one of them must be rejected.
+ h.g.HandleSessionOnOwner(sess, "look")
+
+ // A durable mutation gives an unambiguous admission check: description
+ // input would rewrite the character.
+ sess.State = net.StateChangeDesc
+ h.g.HandleSessionOnOwner(sess, "Input after final save")
+ if sess.Player.Description == "Input after final save" {
+ t.Error("session input still mutates character after shutdown/owner stop")
+ }
+}
+
+func TestShutdownRejectsInputDuringDrain(t *testing.T) {
+ h := newHarness(t)
+ h.seedAccount("alice", "pw", false)
+ h.seedCharacter("alice", "alice")
+ sess, _ := h.connect("alice")
+ sess.State = net.StateGame
+ h.g.setShuttingDown()
+
+ // During the drain window the owner is still alive, but new input is
+ // already rejected: only work accepted before shutdown executes.
+ h.g.HandleSessionOnOwner(sess, "look")
+ sess.State = net.StateChangeDesc
+ h.g.HandleSessionOnOwner(sess, "Input during drain")
+ if sess.Player.Description == "Input during drain" {
+ t.Error("session input accepted after shutdown began")
+ }
+ h.barrier()
+}
diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go
index 7d86cc8..93d186a 100644
--- a/internal/persistence/txn.go
+++ b/internal/persistence/txn.go
@@ -11,6 +11,11 @@ package persistence
// Transactions do NOT serialize concurrent callers: like single-file saves,
// they must be executed by an owner that prevents concurrent mutations of
// the same files (the game's owner goroutine).
+//
+// On apply failure the applied operations are rolled back and the journal is
+// removed. If the ROLLBACK itself fails, the journal is deliberately kept:
+// its before-images are then the only durable record of the pre-transaction
+// state, and the next Recover retries the rollback.
import (
"crypto/rand"
@@ -184,7 +189,13 @@ func (t *Transaction) Commit() error {
if err := t.apply(); err != nil {
if rbErr := t.rollback(); rbErr != nil {
- err = fmt.Errorf("%v; rollback failed: %v", err, rbErr)
+ // The rollback could not restore the before-images: the journal
+ // is now the only durable record of the pre-transaction state.
+ // KEEP it so the next Recover call (or operator repair of the
+ // underlying fault) can retry the rollback instead of silently
+ // losing the before-images forever. See
+ // TestCommitKeepsJournalAfterFailedRollback.
+ return fmt.Errorf("%v; rollback failed (journal kept for recovery: %s): %v", err, journalPath, rbErr)
}
if rErr := os.Remove(journalPath); rErr != nil && !os.IsNotExist(rErr) {
err = fmt.Errorf("%v; journal cleanup failed: %v", err, rErr)
diff --git a/internal/persistence/txn_rollback_test.go b/internal/persistence/txn_rollback_test.go
new file mode 100644
index 0000000..2bd7974
--- /dev/null
+++ b/internal/persistence/txn_rollback_test.go
@@ -0,0 +1,102 @@
+package persistence
+
+// Regression: when a transaction's apply fails AND its rollback fails, the
+// journal is the only durable record of the pre-transaction before-images.
+// Deleting it at commit time made the corruption permanent; it must be kept
+// so the next Recover call (or operator repair) can retry the rollback.
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+func TestCommitKeepsJournalAfterFailedRollback(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "character.yaml")
+ if err := os.WriteFile(path, []byte("before"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ journalDir := filepath.Join(dir, "journals")
+ txn := NewTransaction(journalDir)
+ if err := txn.Replace(path, []byte("after"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ // Deterministically force both the apply and the rollback writes to
+ // fail after the snapshot: the target becomes a nonempty directory,
+ // so WriteFile cannot create the file and rollback cannot restore it.
+ if err := os.Remove(path); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Mkdir(path, 0700); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(path, "blocker"), []byte("x"), 0600); err != nil {
+ t.Fatal(err)
+ }
+
+ err := txn.Commit()
+ if err == nil || !strings.Contains(err.Error(), "rollback failed") {
+ t.Fatalf("need an apply+rollback failure, got: %v", err)
+ }
+ entries, err := ListJournalFiles(journalDir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(entries) == 0 {
+ t.Fatal("failed rollback deleted the only durable before-image; restart cannot recover the pre-transaction state")
+ }
+}
+
+// TestRecoverRetriesKeptRollbackJournal: after the operator repairs the
+// fault that broke the rollback, the next Recover completes it.
+func TestRecoverRetriesKeptRollbackJournal(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "character.yaml")
+ if err := os.WriteFile(path, []byte("before"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ journalDir := filepath.Join(dir, "journals")
+ txn := NewTransaction(journalDir)
+ if err := txn.Replace(path, []byte("after"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Remove(path); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Mkdir(path, 0700); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(path, "blocker"), []byte("x"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ if err := txn.Commit(); err == nil {
+ t.Fatal("expected the injected apply/rollback failure")
+ }
+
+ // Operator repair: put the pre-transaction file back.
+ if err := os.RemoveAll(path); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, []byte("after"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ if err := Recover(journalDir); err != nil {
+ t.Fatalf("recovery retry failed: %v", err)
+ }
+ data, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(data) != "before" {
+ t.Fatalf("recovery did not roll back to the before-image: %q", data)
+ }
+ entries, err := ListJournalFiles(journalDir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(entries) != 0 {
+ t.Fatalf("completed recovery left journals behind: %v", entries)
+ }
+}
diff --git a/internal/persistence/txn_test.go b/internal/persistence/txn_test.go
index 10a809d..796cd56 100644
--- a/internal/persistence/txn_test.go
+++ b/internal/persistence/txn_test.go
@@ -99,8 +99,31 @@ func TestTxnRollbackOnFailure(t *testing.T) {
if got := readTestFile(t, filepath.Join(sub, "gone.yaml")); got != "old-gone" {
t.Fatalf("rollback did not restore deleted file: %q", got)
}
+ // The restore of gone.yaml ran inside the still read-only directory, so
+ // the ROLLBACK itself failed there: the journal is the only durable
+ // record of the pre-transaction state and must be KEPT (deleting it
+ // would make any remaining divergence unrecoverable). See
+ // TestCommitKeepsJournalAfterFailedRollback.
+ if entries, _ := ListJournalFiles(journalDir); len(entries) != 1 {
+ t.Fatalf("journal must be kept after a failed rollback: %v", entries)
+ }
+
+ // Repair the directory permission: the next Recover completes the
+ // rollback and cleans the journal.
+ if err := os.Chmod(sub, 0700); err != nil {
+ t.Fatal(err)
+ }
+ if err := Recover(journalDir); err != nil {
+ t.Fatalf("recovery retry after repair: %v", err)
+ }
+ if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "old-a" {
+ t.Fatalf("recovery retry disturbed a.yaml: %q", got)
+ }
+ if got := readTestFile(t, filepath.Join(sub, "gone.yaml")); got != "old-gone" {
+ t.Fatalf("recovery retry disturbed gone.yaml: %q", got)
+ }
if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 {
- t.Fatalf("journal not cleaned after rollback: %v", entries)
+ t.Fatalf("completed recovery left journals behind: %v", entries)
}
}
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 12:13:21 +0000