N6/C3 review fixes: close the killed-boss-resolves-as-survived window

Two findings from an adversarial review of the C3 diff, both fixed:

1. (medium) A killing blow commits the shared pool to 0 inline, but the status
   flip to 'defeated' was deferred until AFTER the multi-second narration stream.
   In that window a crash/redeploy or the ticker's survive path could resolve a
   boss the town KILLED as a survival — debiting the pot and paying no bounties.
   Fix: resolve the defeat BEFORE streaming narration, and add a ticker safety
   net that resolves any active 0-HP boss as defeated (never falls through to the
   survive/pot-debit branch). Both guarded by setWorldBossStatus, so still once.

2. (minor) Pool damage was applied before the best-effort contrib/gate write, so
   a failed upsert let a player refight a pool they had already drained and lose
   the credit. Fix: write the contribution (which sets the once-per-day gate)
   BEFORE draining the pool, as a hard error that aborts the bout with the pool
   untouched.

Also corrected the applyWorldBossDamage comment (two concurrent killers CAN both
see killed=true; the status guard dedupes the payout — the old comment claimed
only one would). New ticker tests cover both resolution paths; suite green,
golden byte-identical.

Claude-Session: https://claude.ai/code/session_017mEwUmmS7aQTP2NQXj6rUa
This commit is contained in:
prosolis
2026-07-10 18:09:11 -07:00
parent 127d252982
commit e2c0c3359b
2 changed files with 76 additions and 11 deletions

View File

@@ -156,10 +156,11 @@ func insertWorldBoss(name string, tier, hpMax int, startsAt, endsAt time.Time) (
// applyWorldBossDamage subtracts damage from the shared pool atomically, clamped // applyWorldBossDamage subtracts damage from the shared pool atomically, clamped
// at zero, only while the boss is still active. Returns the pool's new value and // at zero, only while the boss is still active. Returns the pool's new value and
// whether this hit felled it (crossed to zero on this call). A concurrent hit // whether the pool is now down (killed). The UPDATE and the re-read are separate
// that also crosses zero will see killed=false — only the writer that moved the // statements, so two concurrent killers can BOTH observe killed=true — that is
// pool to exactly zero, or found it already there, owns resolution; the caller // fine: resolveWorldBossDefeated dedupes on the status='active' guard in
// re-reads to decide. // setWorldBossStatus, so only one payout ever fires. killed is just "should the
// caller attempt resolution".
func applyWorldBossDamage(bossID int64, dmg int) (remaining int, killed bool, err error) { func applyWorldBossDamage(bossID int64, dmg int) (remaining int, killed bool, err error) {
if dmg < 0 { if dmg < 0 {
dmg = 0 dmg = 0
@@ -409,6 +410,14 @@ func (p *AdventurePlugin) worldBossTick() {
} }
now := time.Now().UTC() now := time.Now().UTC()
if boss != nil { if boss != nil {
// Safety net: a killing blow commits the pool to 0 inline, but if the
// process died (or was redeployed) before the bout resolved the status,
// the boss is left active/0-HP. Resolve it as the defeat it was — never
// let the survive branch below debit the pot for a boss the town killed.
if boss.HPCurrent <= 0 {
p.resolveWorldBossDefeated(boss)
return
}
if now.After(boss.EndsAt) { if now.After(boss.EndsAt) {
p.resolveWorldBossSurvived(boss) p.resolveWorldBossSurvived(boss)
} }
@@ -626,6 +635,14 @@ func (p *AdventurePlugin) fightWorldBoss(ctx MessageContext) error {
return p.SendDM(ctx.Sender, "The Siege combat hit an error. Try again in a moment.") return p.SendDM(ctx.Sender, "The Siege combat hit an error. Try again in a moment.")
} }
// Resolve a defeat BEFORE streaming the (multi-second) narration. The pool is
// already committed to 0; flipping the status now closes the window where a
// crash or the ticker's survive path could resolve a killed boss as a
// survival and debit the pot instead of paying bounties.
if bout.Killed {
p.resolveWorldBossDefeated(boss)
}
playerName, _ := loadDisplayName(ctx.Sender) playerName, _ := loadDisplayName(ctx.Sender)
if playerName == "" { if playerName == "" {
playerName = "You" playerName = "You"
@@ -646,10 +663,6 @@ func (p *AdventurePlugin) fightWorldBoss(ctx MessageContext) error {
} }
<-p.sendZoneCombatMessages(ctx.Sender, phaseMessages, footer) <-p.sendZoneCombatMessages(ctx.Sender, phaseMessages, footer)
if bout.Killed {
p.resolveWorldBossDefeated(boss)
}
return nil return nil
} }
@@ -688,13 +701,18 @@ func (p *AdventurePlugin) resolveWorldBossBout(userID id.UserID, boss *worldBoss
if dmg < 0 { if dmg < 0 {
dmg = 0 dmg = 0
} }
// Record the contribution (which also sets the once-per-day gate) BEFORE
// draining the shared pool. If this write fails we must not have already
// drained the pool — otherwise the player could refight a pool they emptied
// and lose the credit for it. A hard error here aborts the bout; the HP cost
// was already paid, but the pool is untouched and the player can retry.
if err := upsertWorldBossContrib(boss.ID, userID, dmg, dateKey); err != nil {
return worldBossBoutResult{}, err
}
remaining, killed, err := applyWorldBossDamage(boss.ID, dmg) remaining, killed, err := applyWorldBossDamage(boss.ID, dmg)
if err != nil { if err != nil {
return worldBossBoutResult{}, err return worldBossBoutResult{}, err
} }
if err := upsertWorldBossContrib(boss.ID, userID, dmg, dateKey); err != nil {
slog.Warn("worldboss: contrib upsert failed", "user", userID, "err", err)
}
return worldBossBoutResult{ return worldBossBoutResult{
Damage: dmg, Remaining: remaining, Killed: killed, Battered: battered, Combat: result, Damage: dmg, Remaining: remaining, Killed: killed, Battered: battered, Combat: result,
}, nil }, nil

View File

@@ -368,3 +368,50 @@ func TestResolveWorldBossBout_FellsTinyPool(t *testing.T) {
t.Errorf("bout should not self-resolve; status=%q", after.Status) t.Errorf("bout should not self-resolve; status=%q", after.Status)
} }
} }
// TestWorldBossTick_ResolvesZeroHPBossAsDefeated: a killing blow that committed
// the pool to 0 but (crash/redeploy) never resolved the status must be resolved
// as DEFEATED by the ticker net — never fall through to the survive/pot-debit
// path. No contributors here, so resolution never touches euro.
func TestWorldBossTick_ResolvesZeroHPBossAsDefeated(t *testing.T) {
newWorldBossTestDB(t)
now := time.Now().UTC()
communityPotAdd(1000)
bossID, err := insertWorldBoss("Ghost", 3, 100, now, now.Add(worldBossWindow))
if err != nil {
t.Fatal(err)
}
if _, err := db.Get().Exec(`UPDATE world_boss SET hp_current = 0 WHERE id = ?`, bossID); err != nil {
t.Fatal(err)
}
p := &AdventurePlugin{}
p.worldBossTick()
after, _ := loadWorldBoss(bossID)
if after.Status != "defeated" {
t.Errorf("status = %q, want defeated (ticker safety net)", after.Status)
}
if bal := communityPotBalance(); bal != 1000 {
t.Errorf("pot = %d, want 1000 — a defeat mints, it must not debit the pot", bal)
}
}
// TestWorldBossTick_ResolvesLapsedBossAsSurvived: a boss whose window lapsed with
// the pool still up survives and loots the pot.
func TestWorldBossTick_ResolvesLapsedBossAsSurvived(t *testing.T) {
newWorldBossTestDB(t)
now := time.Now().UTC()
communityPotAdd(1000)
bossID, err := insertWorldBoss("Titan", 4, 500, now.Add(-worldBossWindow), now.Add(-time.Minute))
if err != nil {
t.Fatal(err)
}
p := &AdventurePlugin{}
p.worldBossTick()
after, _ := loadWorldBoss(bossID)
if after.Status != "survived" {
t.Errorf("status = %q, want survived", after.Status)
}
if bal := communityPotBalance(); bal != 800 {
t.Errorf("pot = %d, want 800 after 20%% tribute", bal)
}
}