Review follow-ups A + B: armed abilities survive the fight, supply pool serialized

A. An armed ability lasted one round of a turn-based fight.

buildZoneCombatants called applyArmedAbility, which applies an ability's mods
*and* clears ArmedAbility and saves the sheet. The turn engine calls that
builder again on every !attack / !cast / !consume, so round 1 fired the ability
and disarmed the character, and every later round rebuilt them with none of its
mods. A Berserker paid stamina for a single round of BerserkerRage /
RageMeleeDmg / PhysicalResistRage / FrenzyDmgBonus. Every entry in
dndActiveAbilities had the same shape. mods.BerserkerRage was not merely unread
at close-out — by then it no longer existed.

Split arming into its two halves:

  consumeArmedAbility(c)          mutates: disarms, saves, returns the id. Once,
                                  at fight start.
  applyAbilityByID(c, id, mods)   pure: no DB write, no disarm. Safe on every
                                  rebuild. (No ability's Apply writes to the
                                  character, so this really is pure.)
  armAbilityForFight(c, mods)     consume + apply, for the auto-resolve callers
                                  that build and fight in one breath.

buildZoneCombatants now takes the already-consumed id and re-applies it. The id
rides on ActorStatuses.ArmedAbility, seeded per seat at fight start, so
partyCombatantsForSession reproduces the ability every rebuild and the close-out
can still see that a rage fired.

The close-out itself: postCombatBookkeeping now carries grantCombatAchievements
+ persistDnDPostCombatSubclass, and all four close-outs route through it —
runDungeonCombat, runZoneCombatRoster, finishCombatSession,
finishPartyCombatSession. It fires on every terminal status, not just a win: a
Berserker who rages and loses is still exhausted, which is what auto-resolve
always did.

Also: buildFightSeats and runZoneCombatRoster consumed the ability before the
checks that could sit a seat out, so a downed member was disarmed for a fight
they never joined. The refusals now run first.

B. Six unlocked read-modify-writes against the shared supply pool.

updateSupplies rewrites supplies_json wholesale, so a caller folding its delta
onto an *Expedition it read earlier discards whatever landed in between.
Handlers run one goroutine per event, so those writers genuinely interleave.

All six now go through withExpeditionSupplies, which takes advExpeditionLock,
re-reads the row, hands the closure the fresh copy and persists what it returns:
nightRolloverBurn (forage + burn in one write), grantTwoWeeksCache,
advanceToNextRegion's transit burn, campPitch, pitchAutopilotCamp, and the
ambient pack-rat drain. expeditionCmdAccept's hand-rolled lock folds onto the
same helper. expedition_sim.go is left alone: single-threaded, takes no locks.

Known consequence, for the balance track: trySimAutoArm used to live inside the
rebuild, so a simulated Fighter (second_wind) or Cleric (healing_word) re-armed
and re-spent a resource every round of every elite/boss fight. expedition-sim
drives those through the turn engine, so every prior expedition-sim corpus
overstates those two classes. Re-baseline after this, not before.

Claude-Session: https://claude.ai/code/session_017mEwUmmS7aQTP2NQXj6rUa

internal/plugin/dnd_abilities.go

@@ -410,22 +410,52 @@ func trySimAutoArm(c *DnDCharacter) string {
 	return ab.Name
 }
 
-// applyArmedAbility checks for a pre-armed ability on the character and
-// applies its effect to the player's CombatModifiers, then clears the armed
-// flag. Called from combat_bridge.go before SimulateCombat.
-func applyArmedAbility(c *DnDCharacter, mods *CombatModifiers) (string, bool) {
+// consumeArmedAbility disarms the character, returning the ability id that was
+// armed. It is the *mutating* half of arming: the flag is cleared and saved, so
+// it must run exactly once per fight, at fight start.
+//
+// It is split from applyAbilityByID because a turn-based fight rebuilds its
+// combatants from the DB on every player command. A rebuild that consumed would
+// fire the ability on round 1, clear the flag, and then hand every later round a
+// character with no ability at all — the player pays the resource for one round
+// of a buff that is supposed to span the fight. So the fight consumes once and
+// persists the id on the session; each rebuild re-applies it from there.
+//
+// An id that is no longer in the ability table is disarmed and reported as "".
+func consumeArmedAbility(c *DnDCharacter) string {
 	if c == nil || c.ArmedAbility == "" {
+		return ""
+	}
+	armed := c.ArmedAbility
+	c.ArmedAbility = ""
+	_ = SaveDnDCharacter(c)
+	if _, ok := dndActiveAbilities[armed]; !ok {
+		return ""
+	}
+	return armed
+}
+
+// applyAbilityByID folds an ability's effect into a freshly-derived set of
+// CombatModifiers. It is pure with respect to persistence — no DB write, no
+// disarm — so it is safe to call on every rebuild of an in-flight fight.
+//
+// abilityID is what consumeArmedAbility returned at fight start. Empty (nobody
+// armed anything) and unknown ids are both no-ops.
+func applyAbilityByID(c *DnDCharacter, abilityID string, mods *CombatModifiers) (string, bool) {
+	if c == nil || abilityID == "" {
 		return "", false
 	}
-	ab, ok := dndActiveAbilities[c.ArmedAbility]
+	ab, ok := dndActiveAbilities[abilityID]
 	if !ok {
-		c.ArmedAbility = ""
-		_ = SaveDnDCharacter(c)
 		return "", false
 	}
 	ab.Apply(c, mods)
-	firedName := ab.Name
-	c.ArmedAbility = ""
-	_ = SaveDnDCharacter(c)
-	return firedName, true
+	return ab.Name, true
+}
+
+// armAbilityForFight consumes whatever the character armed and applies it in one
+// step, for the auto-resolve callers that build a combatant and immediately
+// fight with it. A turn-based fight must not use this — see consumeArmedAbility.
+func armAbilityForFight(c *DnDCharacter, mods *CombatModifiers) (string, bool) {
+	return applyAbilityByID(c, consumeArmedAbility(c), mods)
 }