Five bugs found reviewing n1-restoration end to end.
beginCombatTurn settles any phase the engine owes before reading whose turn it
is. That settle can end the fight — and the old code then answered "you're not
in a fight" and returned. The terminal status was already persisted, so nothing
ever paid the party out: no XP, no loot, no death recorded, no run teardown. The
reaper cannot recover it either, because listExpiredCombatSessions filters on
status='active'. Close the fight out there, the way the !fight start path and
the reaper already do.
A party member was permanently soft-locked when their leader extracted and never
resumed. seatedExpeditionFor (the guard) spans 'extracting'; expeditionForMember
(what !expedition leave resolved through) saw only 'active'. So the member was
refused any new adventure by the guard and told "No active expedition" by the
command the guard points them at, with nothing sweeping stale rows and only the
leader able to clear one. Resolve the exit through the same lookup as the gate.
updateSupplies overwrites supplies_json wholesale, and expeditionCmdAccept folded
a member's packs onto a snapshot read before the coin debit, unlocked. Handlers
run one goroutine per event, so two invitees accepting genuinely interleave and
one member's packs vanish. advUserLock cannot help — it is keyed by sender, so
racing members take different mutexes. Add advExpeditionLock and re-read the pool
under it. Closes accept-vs-accept; the six other updateSupplies callers still
race and are written up separately.
runHarvestInterrupt picked an elite enemy and elite narration off a local `elite`
flag, then passed a hardcoded false as isElite to closeOutZoneWin. dropZoneLoot
gates masterwork on isBoss||isElite, so beating an elite interrupt skipped the
masterwork roll and took standard treasure weight — while the same elite fought
via !zone paid out correctly.
arenaSeasonRollover marked its job complete even when recordArenaSeasonTitle
failed, and JobCompleted short-circuits every later run for that quarter, so a
transient SQLite BUSY lost the crown forever. Defer completion on failure; the
insert is ON CONFLICT DO NOTHING against PRIMARY KEY (season, kind) and a past
season's data is frozen, so the retry is safe.
Also: drop dead partySurvivors, collapse the zoneCombatRoster alias into
fightRoster, route partyCasualtyLine through joinNames, fold four copies of the
expedition column projection into expeditionSelectCols, stop replyDM sending a
blank DM, and correct two doc comments describing a path that no longer exists.
Deliberately not fixed, with reasons, in gogobee_code_review_followups.md — most
notably that both turn-based close-outs skip grantCombatAchievements and
persistDnDPostCombatSubclass, which the auto-resolve paths run.
Every ownership lookup in the adventure module keys on a user id, and a
party member owns neither the expedition row nor the zone run. So each
player-facing read quietly told them they were not playing.
Rewire them. Reads a member should see resolve through activeExpeditionFor
/ activeZoneRunFor. Leader-only actions answer with copy that names the
leader instead of denying the expedition. Three busy-guards had to start
refusing a member outright: !zone enter, !expedition start and !sell all
keyed on the sender's own row, so a seated member could open a private
dungeon, outfit a rival expedition, or run a shop from the boss room.
Four things the rewire itself exposed:
!resources looks like a read but seed-persists harvest nodes, and
saveHarvestNodes rewrites the entire region_state blob — kills, event
gates, temporal stack — last-write-wins. Reaching it as a member would
revert the leader's walk from a stale snapshot. Persist only for the owner;
seedRoomNodes is pure, so a member re-derives the same nodes.
!zone taunt moves the party's shared mood, which is intended and safe:
applyMoodEvent lands an atomic delta. Its neighbour applyMoodDecayIfStale
writes an absolute gm_mood from the caller's snapshot, and every command
takes the *sender's* lock — a member running it against the leader's run
holds the wrong mutex. The owner check now lives on that function.
A seat outlives status='active'. releaseParty deliberately skips the
seven-day 'extracting' limbo, so the roster persists while
activeExpeditionFor goes blind — long enough for a member to open a run
that wins every lookup once the leader !resumes. seatedExpeditionFor spans
both statuses; it is what the busy-guards ask.
!expedition run was still member-blind. It is the same walk as !zone
advance, reached by its other name.
isPartyMember replaces `run != nil && !isLeader`: activeZoneRunFor reports
isLeader=false for a player with no run anywhere, so the bare test sends a
solo player to go ask their leader.
Golden byte-identical; solo T1 expedition clears end-to-end.
Every ownership lookup in the adventure module keys on a user id, and a
party member owns no row: not the expedition, not the zone run. P4 gave
them activeExpeditionFor; this gives them activeZoneRunFor, and gives the
DM seams the audience they never had.
- activeZoneRunFor(user) -> (run, isLeader, err). An owner's lookup is
exactly getActiveZoneRun, side effects and all -- in particular the
§4.3 idle reap, which force-extracts the wrapping expedition. A member
must never re-enter it, or glancing at the map would end the leader's
run. Pinned.
- expeditionAudience / fanOutExpeditionDM. Briefing, recap and digest all
DM'd id.UserID(e.UserID) alone. They now loop the roster, which
partyMemberIDs collapses to exactly the owner when there is none -- so
a solo expedition sends the same bytes to the same user it always has.
The briefing's body is expedition-scoped but its pet prefix is not:
each member has their own pet and their own sheet, so the roll rides a
per-reader decorator (the shape P5 settled on for combat narration).
The digest's A6 event anchor rolls per member for the same reason.
- releaseParty on every terminal transition. A seated member is barred
from adventuring elsewhere, so a roster outliving its expedition
strands the party. Deliberately NOT on 'extracting': that is a 7-day
resumable limbo and !resume must bring everyone back. The roster clears
when the window lapses to 'failed', which routes through
completeExpedition like the rest.
Rosters are still empty in production -- nothing seats a member yet -- so
every loop here has exactly one element and the whole change is a no-op
until P6b. Golden byte-identical, go test ./... green.
