fix: harden economy recovery, shutdown concurrency, world validation, and inventory-loss paths

15 files changed, 198 insertions, 40 deletions

diff --git a/internal/game/account_txn.go b/internal/game/account_txn.go
index 2a420ca..cf31dea 100644
--- a/internal/game/account_txn.go
+++ b/internal/game/account_txn.go
@@ -39,15 +39,22 @@ func (g *Game) newTxn() *persistence.Transaction {
 // first connection and are re-saved so subsequent authorizations can use
 // them. Called from the owner goroutine only.
 func (g *Game) adoptCharacterIdentity(p *player.Player, accountName string) {
+	assignedID := ""
 	if p.ID == "" {
 		p.ID = newCharacterID()
+		assignedID = p.ID
 	}
 	if p.Account == "" {
 		p.Account = accountName
 	}
 	if err := g.AccountStore.SaveCharacter(p); err != nil {
-		// Identity backfill is best-effort: the character remains usable,
-		// but membership validation falls back to the account list.
+		// Identity backfill is best-effort, but a NON-DURABLE ID must never
+		// become an economy key: a later startup's backfill would assign a
+		// different ID and strand every journal record written under this
+		// one. Fall back to the durable character name instead.
+		if assignedID != "" {
+			p.ID = ""
+		}
 		log.Printf("identity backfill for %s: %v", p.Name, err)
 	}
 }
diff --git a/internal/game/act_combine.go b/internal/game/act_combine.go
index b16cf7a..014dbef 100644
--- a/internal/game/act_combine.go
+++ b/internal/game/act_combine.go
@@ -134,6 +134,10 @@ func (g *Game) advanceCombine(sess *net.Session, p *player.Player) {
 		for _, bp := range byproducts {
 			if slot := p.FirstFreeSlot(); slot >= 0 {
 				p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1})
+			} else {
+				// Never silently delete a byproduct: drop the remainder on
+				// the ground where the player can pick it up.
+				g.World.AddGroundItem(p.RoomID, bp, 1)
 			}
 		}
 
diff --git a/internal/game/act_farm.go b/internal/game/act_farm.go
index 91384a0..1e576c3 100644
--- a/internal/game/act_farm.go
+++ b/internal/game/act_farm.go
@@ -180,6 +180,13 @@ func (g *Game) advancePlant(sess *net.Session, p *player.Player) {
 	seedID := data.SeedID
 	xp := data.XP
 
+	if p.CountItem(seedID) < 1 {
+		sess.WriteLine("You no longer have that seed.")
+		p.Action = nil
+		return
+	}
+	p.RemoveItem(seedID, 1)
+
 	g.ensureFarmState(p, prefix)
 
 	g.setPlayerFlag(p, prefix+"_weeds", false)
diff --git a/internal/game/cmd_farm.go b/internal/game/cmd_farm.go
index dec16c0..1c8f4c8 100644
--- a/internal/game/cmd_farm.go
+++ b/internal/game/cmd_farm.go
@@ -124,7 +124,8 @@ func (g *Game) doPlant(sess *net.Session, input string) {
 		return
 	}
 
-	p.RemoveItem(seedID, 1)
+	// The seed is consumed when the plant action COMPLETES (advancePlant),
+	// not here: an interrupted action must not destroy the seed.
 
 	p.Action = &behavior.Action{
 		Type:       "plant",
diff --git a/internal/game/cmd_quit.go b/internal/game/cmd_quit.go
index d6e3313..f8075c1 100644
--- a/internal/game/cmd_quit.go
+++ b/internal/game/cmd_quit.go
@@ -89,7 +89,11 @@ func (g *Game) beginQuitCountdown(sess *net.Session) {
 			if g.Hub != nil {
 				g.Hub.LeaveRoom(sess)
 			}
-			sess.Player = nil
+			if g.Hub != nil {
+				g.Hub.SetSessionPlayer(sess, nil)
+			} else {
+				sess.Player = nil
+			}
 			sess.SetState(net.StateMenu)
 			g.showMenu(sess)
 			return false
diff --git a/internal/game/cmd_shutdown.go b/internal/game/cmd_shutdown.go
index 0030649..9a57c6b 100644
--- a/internal/game/cmd_shutdown.go
+++ b/internal/game/cmd_shutdown.go
@@ -105,9 +105,16 @@ func (g *Game) broadcastLoggedIn(msg string) {
 	if g.Hub == nil {
 		return
 	}
-	for _, other := range g.Hub.AllSessions() {
-		if other.Player != nil {
-			other.WriteLine(msg)
+	// Player pointers and option maps are owner-owned. Route the broadcast
+	// through the owner so it cannot race a login/logout or an option change
+	// from a raw shutdown goroutine (a concurrent map read/write would fatal
+	// the process before the final save). Drops harmlessly once the owner has
+	// stopped.
+	g.run(func() {
+		for _, other := range g.Hub.AllSessions() {
+			if other.Player != nil {
+				other.WriteLine(msg)
+			}
 		}
-	}
+	})
 }
diff --git a/internal/game/cmd_trigger_combat.go b/internal/game/cmd_trigger_combat.go
index f9a6bf1..0578c2b 100644
--- a/internal/game/cmd_trigger_combat.go
+++ b/internal/game/cmd_trigger_combat.go
@@ -64,9 +64,19 @@ func (g *Game) scienceAttack(sess *net.Session, p *player.Player, mob *world.Mob
 		if mob.HP < 0 {
 			mob.HP = 0
 		}
+		// Science damage obeys the same finishing-blow rule as melee/ranged:
+		// a mob with a required finisher cannot be killed by raw damage.
+		if mob.FinishingBlow != "" && mob.HP <= 0 {
+			mob.HP = 1
+		}
 		if mob.HP < mob.MaxHP && mob.HP > 0 {
 			mob.StartRegen()
 		}
+		if mob.FinishingBlow != "" && mob.HP == 1 {
+			sess.WriteLine(g.colorize(sess, "warning",
+				fmt.Sprintf("%s resists death! Use %s on it to finish it off.",
+					mobDisplayName(mob, false), g.itemDisplayName(mob.FinishingBlow))))
+		}
 
 		sciXP := mod.BaseXP
 		hpXP := mod.BaseXP / 3
diff --git a/internal/game/cmd_trigger_enchant.go b/internal/game/cmd_trigger_enchant.go
index 62a3c48..02c9755 100644
--- a/internal/game/cmd_trigger_enchant.go
+++ b/internal/game/cmd_trigger_enchant.go
@@ -72,23 +72,22 @@ func (g *Game) triggerChipBolts(sess *net.Session, p *player.Player, mod *ModDef
 		return
 	}
 
+	outputDef, _ := g.ItemStore.Load(chip.Output)
+	// Verify the output can be placed BEFORE consuming the bolts: never
+	// consume inputs into an inventory with nowhere to put the result.
+	if !craftOutputFits(p, chip.Output, outputDef) {
+		sess.WriteLine("Your inventory is too full!")
+		return
+	}
+
 	if p.Action != nil {
 		g.cancelAction(p)
 	}
 
 	p.RemoveItem(chip.Input, chip.Qty)
-	placed := false
-	for i := 0; i < 28; i++ {
-		slot := p.InvSlot(i)
-		if slot != nil && slot.ItemID == chip.Output {
-			slot.Quantity += chip.Qty
-			placed = true
-			break
-		}
-	}
-	if !placed {
-		freeSlot := p.FirstFreeSlot()
-		p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: chip.Output, Quantity: chip.Qty})
+	if !placeCraftOutput(p, chip.Output, outputDef, chip.Qty) {
+		sess.WriteLine("Your inventory is too full!")
+		return
 	}
 
 	g.triggerModReward(sess, p, mod, 1.0)
diff --git a/internal/game/cmd_trigger_utility.go b/internal/game/cmd_trigger_utility.go
index 09eabb1..b5a168b 100644
--- a/internal/game/cmd_trigger_utility.go
+++ b/internal/game/cmd_trigger_utility.go
@@ -136,19 +136,52 @@ func (g *Game) triggerEmGrab(sess *net.Session, p *player.Player, mod *ModDef, t
 	}
 
 	qty := groundItems[matchedID]
-	g.World.RemoveGroundItem(p.RoomID, matchedID, qty)
-
 	def, _ := g.ItemStore.Load(matchedID)
 	name := matchedID
 	if def != nil {
 		name = def.Name
 	}
 
-	freeSlot := p.FirstFreeSlot()
-	p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: matchedID, Quantity: qty})
+	// Credits are currency, not an inventory item: collect them the same way
+	// the get command does.
+	if matchedID == "credits" {
+		g.World.RemoveGroundItem(p.RoomID, matchedID, qty)
+		g.grantCredits(p, qty)
+		g.triggerModReward(sess, p, mod, 1.0)
+		sess.WriteLine(fmt.Sprintf("You magnetically pull %d credits toward you.", qty))
+		return
+	}
 
-	g.triggerModReward(sess, p, mod, 1.0)
+	if def != nil && def.Stackable {
+		g.World.RemoveGroundItem(p.RoomID, matchedID, qty)
+		if !placeCraftOutput(p, matchedID, def, qty) {
+			g.World.AddGroundItem(p.RoomID, matchedID, qty)
+			sess.WriteLine("Your inventory is full.")
+			return
+		}
+	} else {
+		// Non-stackable items take one slot each; leave any remainder on the
+		// ground instead of cramming N items into a single slot.
+		taken := 0
+		for taken < qty {
+			freeSlot := p.FirstFreeSlot()
+			if freeSlot < 0 {
+				break
+			}
+			p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: matchedID, Quantity: 1})
+			taken++
+		}
+		if taken == 0 {
+			sess.WriteLine("Your inventory is full.")
+			return
+		}
+		g.World.RemoveGroundItem(p.RoomID, matchedID, taken)
+		if taken < qty {
+			sess.WriteLine(fmt.Sprintf("Your inventory fills up; %d %s remain on the ground.", qty-taken, name))
+		}
+	}
 
+	g.triggerModReward(sess, p, mod, 1.0)
 	sess.WriteLine(fmt.Sprintf("You magnetically pull the %s toward you.", g.itemColorize(sess, def, name)))
 }
 
@@ -214,6 +247,13 @@ func (g *Game) triggerSuperheat(sess *net.Session, p *player.Player, mod *ModDef
 		g.cancelAction(p)
 	}
 
+	// Verify the output can be placed BEFORE consuming: a full inventory
+	// must never destroy ingredients without producing the bar.
+	if !craftOutputFits(p, match.ID, match) {
+		sess.WriteLine("Your inventory is too full!")
+		return
+	}
+
 	consumeIngredients(c, p.CountItem, p.RemoveItem)
 
 	outputQty := c.OutputQty
@@ -288,19 +328,30 @@ func (g *Game) triggerPlankMake(sess *net.Session, p *player.Player, mod *ModDef
 			return
 		}
 
+		plankDef, _ := g.ItemStore.Load(info.plankID)
+		// Check placement BEFORE consuming the log and credits: a full
+		// inventory must never delete materials without producing planks.
+		if !craftOutputFits(p, info.plankID, plankDef) {
+			sess.WriteLine("Your inventory is full.")
+			return
+		}
+
 		if p.Action != nil {
 			g.cancelAction(p)
 		}
 
-		p.RemoveItem(inv.ItemID, 1)
+		logID := inv.ItemID
+		p.RemoveItem(logID, 1)
 		p.Credits -= info.cost
 
-		freeSlot := p.FirstFreeSlot()
-		if freeSlot == -1 {
+		if !placeCraftOutput(p, info.plankID, plankDef, 1) {
+			// Defensive: craftOutputFits guaranteed placement. Refund the
+			// log (to the ground, so it can never be lost) and the credits.
+			p.Credits += info.cost
+			g.World.AddGroundItem(p.RoomID, logID, 1)
 			sess.WriteLine("Your inventory is full.")
 			return
 		}
-		p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: info.plankID, Quantity: 1})
 
 		g.triggerModReward(sess, p, mod, 1.0, xpGain{Skill: string(player.Construction), XP: 10})
 
diff --git a/internal/game/cmd_wear.go b/internal/game/cmd_wear.go
index cea4d67..a02fd3b 100644
--- a/internal/game/cmd_wear.go
+++ b/internal/game/cmd_wear.go
@@ -94,13 +94,14 @@ func (g *Game) doWear(sess *net.Session, input string) {
 
 	if slotOccupied {
 		freeSlot := p.FirstFreeSlot()
-		if freeSlot == -1 && existingItemID != slot.ItemID {
+		// The equipped item must always be returned to the inventory before
+		// the new one is removed; with no free slot, refuse rather than
+		// silently destroy an identical equipped copy.
+		if freeSlot == -1 {
 			sess.WriteLine("Your inventory is full.")
 			return
 		}
-		if freeSlot >= 0 {
-			p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existingItemID, Quantity: 1})
-		}
+		p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existingItemID, Quantity: 1})
 	}
 
 	if slot.Quantity > 1 {
diff --git a/internal/game/core_login_char.go b/internal/game/core_login_char.go
index 70746fc..0f57d3c 100644
--- a/internal/game/core_login_char.go
+++ b/internal/game/core_login_char.go
@@ -104,7 +104,11 @@ func (g *Game) connectCharacter(sess *net.Session, name string) {
 	}
 	g.adoptCharacterIdentity(p, sess.Account.Name)
 
-	sess.Player = p
+	if g.Hub != nil {
+		g.Hub.SetSessionPlayer(sess, p)
+	} else {
+		sess.Player = p
+	}
 	sess.SetState(net.StateGame)
 	g.loggedInChars[name] = sess
 	g.charsMu.Unlock()
diff --git a/internal/game/economy.go b/internal/game/economy.go
index 21e7721..b39dbe4 100644
--- a/internal/game/economy.go
+++ b/internal/game/economy.go
@@ -69,6 +69,10 @@ type economyRecord struct {
 	Credit   int       `json:"credits"`
 	Status   string    `json:"status"` // "open", "settled", "void"
 	TS       time.Time `json:"ts"`
+	// Seq is the record's monotonic creation order within the journal. It
+	// lets coverage checks distinguish a settlement that predates a stake
+	// (and therefore cannot return it) from one written after it.
+	Seq int64 `json:"seq,omitempty"`
 	// Refund marks a payout record as a SHUTDOWN/EVICTION REFUND (the
 	// stake being returned), not a game settlement. Recovery must not
 	// treat the chip part of a refund as paid in credits: a refund's chip
@@ -117,6 +121,9 @@ type economyJournal struct {
 	// (stakeCovered). Same ownership discipline as stakes: all mutations
 	// happen on the game owner or during startup recovery.
 	openPayoutRecs map[string][]*economyRecord
+	// seq is the monotonic record counter (see economyRecord.Seq). It is
+	// advanced by startKind and raised to the journal maximum on every fold.
+	seq int64
 }
 
 func newEconomyJournal(dataDir string) *economyJournal {
@@ -198,6 +205,8 @@ func (j *economyJournal) startKind(kind string, refund bool, covers []string, p
 	}
 	// Retry a bounded number of times: a transient append failure must not
 	// drop the record that makes the wager or owed payout durable.
+	j.seq++
+	rec.Seq = j.seq
 	if err := j.appendRetry(*rec, 5); err != nil {
 		return nil, err
 	}
@@ -225,12 +234,16 @@ func (j *economyJournal) closePayoutTracking(rec *economyRecord) {
 	}
 }
 
-// settle marks a record settled (the balance mutation is durable).
-// Fire-and-forget: callers whose recovery semantics depend on the marker
-// being durable use settleDurable instead.
+// settle marks a record settled (the balance mutation is durable). The
+// marker is retried and the record is left OPEN (with its tracking index
+// intact) if no append succeeds: a non-durable settle must never drop the
+// evidence a later recovery needs, or the liability is paid twice.
 func (j *economyJournal) settle(rec *economyRecord) {
 	rec.Status = "settled"
-	_ = j.append(*rec)
+	if err := j.appendRetry(*rec, 3); err != nil {
+		rec.Status = "open"
+		return
+	}
 	j.closePayoutTracking(rec)
 }
 
@@ -249,10 +262,15 @@ func (j *economyJournal) settleDurable(rec *economyRecord) bool {
 }
 
 // void marks a record void (the mutation was reverted in memory and never
-// became durable; recovery must not compensate).
+// became durable; recovery must not compensate). Like settle, the marker is
+// retried and the record is left open on failure so its index/evidence is
+// preserved for a later attempt.
 func (j *economyJournal) void(rec *economyRecord) {
 	rec.Status = "void"
-	_ = j.append(*rec)
+	if err := j.appendRetry(*rec, 3); err != nil {
+		rec.Status = "open"
+		return
+	}
 	j.closePayoutTracking(rec)
 }
 
@@ -344,6 +362,13 @@ func (j *economyJournal) stakeCovered(stake *economyRecord) bool {
 		if payout.Refund {
 			continue
 		}
+		// A payout written BEFORE the stake cannot be its settlement: without
+		// this ordering check an old open payout (for example a failed slots
+		// win) would mask a later, unrelated stake and RefundOutstanding
+		// would resolve it without returning the money.
+		if payout.Seq <= stake.Seq {
+			continue
+		}
 		if len(payout.Covers) == 0 || containsString(payout.Covers, stake.ID) {
 			return true
 		}
@@ -526,6 +551,9 @@ func (j *economyJournal) loadFolded() ([]foldedRecord, error) {
 		if corruptPending {
 			return nil, fmt.Errorf("economy journal line %d is malformed; later records exist and may be owed money (journal preserved for inspection)", corruptSeq)
 		}
+		if rec.Seq > j.seq {
+			j.seq = rec.Seq
+		}
 		if existing, ok := latest[rec.ID]; ok {
 			existing.rec = rec
 			existing.seq = seq
diff --git a/internal/game/economy_seq_test.go b/internal/game/economy_seq_test.go
new file mode 100644
index 0000000..2c5bbdb
--- /dev/null
+++ b/internal/game/economy_seq_test.go
@@ -0,0 +1,24 @@
+package game
+
+import "testing"
+
+// TestStakeCoveredRespectsCreationOrder: an open payout created BEFORE a stake
+// must not be treated as that stake's settlement, or RefundOutstanding would
+// resolve the later stake without returning its money.
+func TestStakeCoveredRespectsCreationOrder(t *testing.T) {
+	j := newEconomyJournal(t.TempDir())
+	stake := &economyRecord{ID: "stake", Player: "alice", Kind: "wager", Status: "open", Seq: 2}
+	j.registerStake(stake)
+
+	older := &economyRecord{ID: "older", Player: "alice", Kind: "payout", Status: "open", Seq: 1, Tracked: true}
+	j.openPayoutRecs[stake.key()] = []*economyRecord{older}
+	if j.stakeCovered(stake) {
+		t.Fatal("a payout created before the stake must not cover it")
+	}
+
+	newer := &economyRecord{ID: "newer", Player: "alice", Kind: "payout", Status: "open", Seq: 3, Tracked: true}
+	j.openPayoutRecs[stake.key()] = []*economyRecord{newer}
+	if !j.stakeCovered(stake) {
+		t.Fatal("a payout created after the stake should cover it")
+	}
+}
diff --git a/internal/game/game.go b/internal/game/game.go
index fc6914f..ef48b63 100644
--- a/internal/game/game.go
+++ b/internal/game/game.go
@@ -187,6 +187,13 @@ func (g *Game) SetHub(hub *net.Hub) {
 	})
 	hub.OnDisconnect(func(sess *net.Session) {
 		g.run(func() {
+			// During graceful shutdown the shutdown sequence owns final
+			// persistence and refunds outstanding wagers; a disconnect
+			// forfeit here would destroy a stake the shutdown policy
+			// promises to return. Mirrors the OnRemove guard.
+			if g.isShuttingDown() {
+				return
+			}
 			if sess.Player != nil {
 				g.emitCasinoEvents(g.Casino.MarkDisconnected(sess.Player.Name))
 			}
diff --git a/internal/game/production_engine.go b/internal/game/production_engine.go
index 2cd9b1d..035b7e2 100644
--- a/internal/game/production_engine.go
+++ b/internal/game/production_engine.go
@@ -253,6 +253,10 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool {
 		for _, bp := range byproducts {
 			if slot := p.FirstFreeSlot(); slot >= 0 {
 				p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1})
+			} else {
+				// Never silently delete a byproduct: drop the remainder on
+				// the ground where the player can pick it up.
+				g.World.AddGroundItem(p.RoomID, bp, 1)
 			}
 		}