adventure: work the five review findings the last pass left open

The extract pre-check is gone. It read a snapshot up to two minutes behind and
still got the last word, so somebody who set out over Matrix during a lagging
roster push was told they weren't on an expedition for a run gogobee would
happily have ended. Same call abandon and leave already made: let it through and
let rejected_not_running be the answer.

The siege_join check stays, because whether a boss is camped outside town is
town-wide and runs on a day-or-longer clock, but it now reads one column through
SiegeIsCamped instead of loading every defender row and the whole history to
look at one flag.

The war-room history insert is OR REPLACE. boss_id is the primary key and it was
never settled whether gogobee means the siege instance or the boss type by it, so
a duplicate pair used to fail the transaction carrying the live boss and the
muster too and freeze the war room on the last good snapshot. A dropped history
row is the smaller failure; the open question is noted in the schema.

offersToUndo's guard didn't cover the case its comment claimed. A gogobee too old
to push seats sends a valid blob with no party key, which decodes to the same
empty slice as a solo run, and a party member got shown the button that throws
away everyone's day. That needs a new field, so whoDetail gains party_known and
the flag gates the empty-list branch alone; the branch that reads the viewer's
own seat is self-evidencing and keeps working against any sender. gogobee's half
is written up in adventure_party_known_flag.md.

And an empty offer list no longer claims "you're already out there", which Pete
can't actually know from a game box too old to push offers at all.
This commit is contained in:
prosolis
2026-07-24 22:45:26 -07:00
parent c40ac1e673
commit aac6c3e127
9 changed files with 232 additions and 232 deletions
+75
View File
@@ -0,0 +1,75 @@
# Wire contract — `party_known` on the public detail blob (gogobee side)
Status: **Pete side BUILT + TESTED. gogobee side NOT BUILT.** Written 2026-07-24 out
of a code review of the W9 verbs branch. Small: one boolean, set unconditionally,
no new endpoint and no new verdict. Companion to `adventure_expansion_spec.md`.
## The hole
`offersToUndo` in `internal/web/who.go` decides which of "Call the whole thing off"
(abandon) and "Leave the party" (leave) an owner's page offers. On a live expedition
with an EMPTY party list it took the "solo run ⇒ this player is the leader" branch,
because a solo run publishes no seats at all.
But a gogobee too old to push party seats sends a perfectly valid detail blob with no
`party` key, and that decodes to the same empty slice. So a party MEMBER served by an
old game box was shown the leader's button — the one that throws away four people's
day. gogobee refuses it with `rejected_not_leader`, so the live cost was a misleading
offer rather than a lost expedition, but the page was asserting something it could
not know.
Nil and empty cannot be told apart on the current wire, hence a new field.
## What gogobee must send
The public detail blob on the roster push (the one Pete decodes into `whoDetail`)
gains:
```json
"party_known": true
```
**Set it unconditionally**, on every sheet, from any gogobee build that knows what a
party seat is:
- on a solo run — YES, this is the case the flag exists for;
- when the party list is omitted because there is nobody else — YES;
- when the player is standing in town — YES;
- when the sheet does carry seats — YES, harmless there.
It is a capability flag about the **sender**, not a fact about the character. It must
never be computed from whether there is a party, whether the run is shared, or
anything else on the sheet. A conditional `party_known` is worse than none: it would
read as "this player is solo" and put the flag straight back into the hole it was
added to close.
Anything that omits it (old builds, JSON that fails to decode) lands as `false` in
Go, which is the fail-closed answer Pete wants.
## What Pete already does with it
- `whoDetail.PartyKnown bool \`json:"party_known"\`` — `internal/web/who.go:47`.
- `offersToUndo`'s third parameter is `partyKnown`; the call site passes
`page.HasDetail && page.Detail.PartyKnown`, so a blob that failed to decode is
still excluded.
- The flag gates the `len(party) == 0` branch **only**. The `len(party) > 0` branch
reads the viewer's own seat and is self-evidencing — a seat that says "member"
cannot be mistaken for leadership — so it keeps working against every sender,
including ones that never set the flag.
- Cases pinned in `internal/web/orders_undo_test.go`.
## Rollout
Pete is already deployed-safe: it ships first and costs nothing but one button. Until
gogobee sets the flag, a genuine SOLO leader on a live run does not see the abandon
button on the party branch. They are not stuck — the `self.Resume != nil` clause
still offers it for an extracted run from town, and abandoning over Matrix was never
affected. The moment gogobee ships the flag the button comes back with no Pete change.
## Done when
- gogobee sets `party_known: true` on every public detail sheet it builds.
- A solo adventurer on a live expedition sees "Call the whole thing off" on their web
page again.
- A party member on the same build still sees "Leave the party" and never the abandon.
- Delete this file.
-181
View File
@@ -1,181 +0,0 @@
# Code review findings + fix plan — w9-verbs-and-polish
From a `/code-review --fix` pass on the branch diff, 2026-07-24. Two findings were
fixed in that session; the five below were left for a follow-up session, with the
direction chosen by the author. Work them in the order given — items 1/2 and 3
touch different files and are independent, item 4 is the only one with a
gogobee-side half.
Already fixed on this branch (don't redo):
- `internal/web/orders.go` verdict handler — non-`ErrNoSuchAdvOrder` storage
failures now return 500 instead of 400, via a new `storage.ErrBadAdvVerdict`.
A 400 makes gogobee park the row for a human, so a transient SQLite blip used
to permanently strand a resolvable order.
- `internal/web/push_adventure.go``advStoryURL` / `advRunOrStoryURL` now
path-escape the guid and run id like every other URL builder does.
---
## 1. Drop the extract pre-check — `internal/web/orders.go:129`
**Why.** The comment above the pre-checks says they read a snapshot up to two
minutes behind and that "neither is allowed to be the last word", and `who.html`
promises the button works even on an idle-looking mark. But
`if haveEntry && entry.Status != "expedition"` returns 409 and no order is ever
created. A player who set out via Matrix during a lagging roster push (staleness
is tolerated up to 12 minutes) gets told they're not on an expedition for a run
gogobee would happily have ended.
**Decision: drop it.** Same call `abandon` and `leave` already make one function
down — and the reasoning there (an *extracted* expedition is still abandonable
while its owner reads as idle) applies to extract too. Cost accepted: a genuine
mistake now comes back as a verdict rather than an instant 409.
**Do.**
- Delete the `case storage.AdvActionExtract:` arm. Replace it with a comment in
the shape of the `AdvActionAbandon, AdvActionLeave` one in
`resolveAdvOrderParams` — say that the snapshot is stale, that a Matrix
departure can outrun the roster push, and that `rejected_not_running` is the
honest answer.
- With extract gone the `switch req.Action` has one arm left; collapse it to an
`if req.Action == storage.AdvActionSiegeJoin` (see item 2, which rewrites the
body anyway).
- `characterName` is still needed for the insert, so keep the `RosterEntryByToken`
lookup and its error path; only the status test goes.
- Nothing to do client-side: `adventure-actions.js:28` already renders
`rejected_not_running` as "couldn't, you weren't on an expedition", which is
the string the 409 was standing in for.
**Tests.** `TestActionPreChecksAreCourtesyOnly` in `internal/web/orders_test.go`
pins the old behaviour — its first block asserts `extract` while idle is a 409.
Flip it to 200 and rewrite the doc comment, which currently describes an
asymmetry that no longer exists (both halves are now "let it through" except the
one positive siege case). Consider renaming it to match. Its two siege blocks
stay exactly as they are.
## 2. Make the siege_join pre-check cheap — `internal/web/orders.go:135`
**Why.** It calls `storage.LoadSiege()`, which loads every defender row and the
whole siege history, to read one `active` flag, on a pool that is
`MaxOpenConns(1)`. Once per click, so not urgent, but it is a one-column read.
**Keep the check itself.** Unlike a personal status, "is a boss camped outside
town" is a town-wide fact on a day-or-longer clock, so a two-minute-old copy is
almost never wrong about it — the reason item 1 goes the other way doesn't apply.
**Do.**
- Add `SiegeIsCamped() (active, known bool, err error)` to
`internal/storage/siege.go`, next to `LoadSiege`. One
`SELECT active FROM adventure_siege WHERE id = 1`; `sql.ErrNoRows` means
known=false.
- The known/active distinction is load-bearing and already pinned by
`TestActionPreChecksAreCourtesyOnly`: no snapshot at all must queue the order
(a fresh deploy must not have a dead button), only a snapshot that positively
says `active=0` refuses. Keep `LoadSiege`'s doc note about that on the new
function.
- Swap the call in `orders.go`. No behaviour change, so no new test beyond the
existing one continuing to pass.
## 3. Make the war-room history replace collision-proof — `internal/storage/siege.go:116`
**Why.** `adventure_siege_history` has `boss_id` as PRIMARY KEY and `ReplaceSiege`
uses a bare `INSERT`, inside the transaction that also writes the live boss and
the muster. Two history rows sharing a `boss_id` fail the whole replace, so the
war room freezes on the previous snapshot indefinitely with no partial
degradation. It is genuinely unclear whether `boss_id` is the siege instance or
the boss type — `SiegeBarForBoss` matches history on `boss_name` + nearest
`ended_at` and its comment says "the same boss comes back month after month",
which reads like type.
**Decision: don't settle the question, make it survivable.** `INSERT OR REPLACE`
can never lose a row that would otherwise have landed; it just keeps the last of
a colliding pair instead of 500ing the ingest.
**Do.**
- `hstmt` becomes `INSERT OR REPLACE INTO adventure_siege_history (...)`.
- Comment why, at the statement: the whole table is deleted and rebuilt from
gogobee's list every push, so a duplicate key is a wire quirk and not data
loss, and a frozen war room is a worse failure than a dropped history row.
- Also note the open question in `schema.go` above the `boss_id INTEGER PRIMARY
KEY` line, so the next reader knows the key's meaning was never confirmed.
**Tests.** New one in `internal/web/siege_test.go` alongside the existing push
tests: post a snapshot whose history carries two rows with the same `boss_id`,
assert the push succeeds and the war room shows the new live boss — i.e. the
ingest degrades to one history row rather than freezing.
## 4. Close the party-guard hole — `internal/web/who.go:133` (+ gogobee)
**Why.** `offersToUndo` receives `page.HasDetail`, which only means "the blob
decoded". Its comment claims the guard also catches "a gogobee too old to push
seats", but such a gogobee pushes a *valid* blob with no `party` key: `HasDetail`
is true, `Party` is nil, and the code takes the "solo ⇒ this player is the
leader" branch and shows a party **member** "Call the whole thing off" — the
exact outcome the comment says it prevents. gogobee refuses with
`rejected_not_leader`, so today's cost is a misleading offer, not a lost
expedition.
A nil `Party` cannot be told apart from a solo run on the current wire, so this
needs a new field. **Decision: spec the wire change.**
**Note the asymmetry before writing any code:** only the `len(party) == 0` branch
is unsafe. The `len(party) > 0` branch reads the viewer's *own* seat and is
self-evidencing — it cannot mistake a member for a leader. So gate the empty
branch alone, and the rollout costs nothing but a solo leader's party-branch
abandon in the window before gogobee ships (the `self.Resume != nil` branch below
still covers the extracted case).
**Pete side.**
- `whoDetail` gains `PartyKnown bool \`json:"party_known"\`` with a comment
saying what nil-vs-empty could not express.
- `offersToUndo`'s third parameter changes from `haveParty` to `partyKnown`;
pass `page.HasDetail && page.Detail.PartyKnown` at the call site (`who.go:405`)
so a blob that failed to decode is still excluded.
- Move the guard: keep `status == rosterStatusExpedition` on the outer `if`, and
put `partyKnown` on the `len(party) == 0` branch only.
- Rewrite the doc comment — it is currently wrong about what the guard buys, and
the "found by getting a fixture wrong" note should stay but now point at the
real signal.
**gogobee side (separate repo).** Write it up as a contract the way
`adventure_ask7_equipment_mgmt.md` was: the public detail blob on the roster push
sets `"party_known": true` whenever the sheet was built by a gogobee that knows
about party seats — unconditionally, including on a solo run, and including when
the party list is omitted. It is a capability flag about the *sender*, not a fact
about the character, so it must never be conditional on there being a party.
**Tests.** `internal/web/orders_undo_test.go` already tables this function. Add
cases: `partyKnown=false` + empty party + expedition ⇒ no abandon (the old
gogobee); `partyKnown=true` + empty party ⇒ abandon (genuine solo);
`partyKnown=false` + a party naming the viewer as member ⇒ leave still offered
(the self-evidencing branch must not regress).
## 5. Stop asserting a fact Pete can't know — `internal/web/orders.go:210`
**Why.** `resolveAdvOrderParams` treats `len(detail.Zones) == 0` as proof the
player is on an expedition and says "you're already out there", but an empty
offer list is equally what an out-of-date game box sends. Only reachable via a
hand-crafted request today (with no offers the picker doesn't render), so this is
message honesty, not a live bug.
**Do.** Keep the refusal, reword it so it describes what Pete actually saw —
something like "nowhere is on offer for you right now" — and adjust the comment
above the branch, which states the "already out there" reading as fact. Then fix
the two places that echo it: the doc comment on
`TestNoZoneOffersMeansAlreadyOut` (`orders_test.go:331`) and, if you want them to
match, nothing else — `adventure-actions.js:33`'s `rejected_busy` string is
gogobee's own verdict and is correct as it stands.
---
## Wrap-up
- `go build ./...`, `go vet ./...`, `go test ./...` after each item.
- `gofmt -l` already flags `internal/storage/orders.go` on this branch (pre-existing
hand-aligned comment blocks); don't let that fool you into reformatting it.
- Delete this file once the five are done.
+6
View File
@@ -138,6 +138,12 @@ CREATE TABLE IF NOT EXISTS adventure_siege_defenders (
fought_today INTEGER NOT NULL DEFAULT 0 fought_today INTEGER NOT NULL DEFAULT 0
); );
-- Open question, never confirmed with gogobee: whether boss_id identifies the
-- siege instance or the boss TYPE. SiegeBarForBoss matches history on boss_name
-- plus the nearest ended_at and its comment says "the same boss comes back month
-- after month", which reads like a type — in which case this key collides on the
-- second visit. ReplaceSiege inserts OR REPLACE so a collision costs one history
-- row instead of the whole push; settle the meaning before relying on the key.
CREATE TABLE IF NOT EXISTS adventure_siege_history ( CREATE TABLE IF NOT EXISTS adventure_siege_history (
boss_id INTEGER PRIMARY KEY, boss_id INTEGER PRIMARY KEY,
boss_name TEXT NOT NULL, boss_name TEXT NOT NULL,
+26 -1
View File
@@ -114,8 +114,14 @@ func ReplaceSiege(s Siege, snapshotAt int64) error {
} }
} }
// OR REPLACE, because boss_id is the primary key and a duplicate in gogobee's
// list would otherwise fail this whole transaction — the live boss and the
// muster with it, freezing the war room on the previous snapshot indefinitely.
// The table is deleted and rebuilt from the pushed list every time, so a
// collision is a wire quirk rather than data loss, and keeping the last of a
// colliding pair is a far smaller failure than a war room that stops moving.
hstmt, err := tx.Prepare(` hstmt, err := tx.Prepare(`
INSERT INTO adventure_siege_history INSERT OR REPLACE INTO adventure_siege_history
(boss_id, boss_name, tier, outcome, hp_remaining, hp_max, defenders, mvp, mvp_fights, ended_at) (boss_id, boss_name, tier, outcome, hp_remaining, hp_max, defenders, mvp, mvp_fights, ended_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`)
if err != nil { if err != nil {
@@ -174,6 +180,25 @@ func SiegeBarForBoss(boss string, at int64) (current, max int, ok bool) {
return hpCur, hpMax, true return hpCur, hpMax, true
} }
// SiegeIsCamped answers the one question the siege_join pre-check asks, without
// LoadSiege's defender rows and whole history behind it — a one-column read on a
// pool that is MaxOpenConns(1).
//
// known is false when gogobee has never pushed a war room at all, which is NOT
// the same as a pushed snapshot saying no Siege is camped. The caller has to keep
// the two apart: a fresh deploy that has not been pushed to yet must still queue
// the order rather than show a dead button.
func SiegeIsCamped() (active, known bool, err error) {
err = Get().QueryRow(`SELECT active FROM adventure_siege WHERE id = 1`).Scan(&active)
if err == sql.ErrNoRows {
return false, false, nil
}
if err != nil {
return false, false, err
}
return active, true, nil
}
// LoadSiege returns the war room as last pushed. ok is false when gogobee has // LoadSiege returns the war room as last pushed. ok is false when gogobee has
// never pushed one at all — distinct from a pushed snapshot that says no Siege // never pushed one at all — distinct from a pushed snapshot that says no Siege
// is camped, which is a real answer the page can render. // is camped, which is a real answer the page can render.
+24 -14
View File
@@ -111,10 +111,11 @@ func (s *Server) handleAdvOrder(w http.ResponseWriter, r *http.Request) {
return return
} }
// Per-verb pre-checks, all courtesy only. Both read Pete's snapshot copy, which // The roster lookup is for the character name the order carries; the one
// is up to two minutes behind the game box, so neither is authoritative and // surviving pre-check below is courtesy only. Anything read here is Pete's
// neither is allowed to be the last word — a run that ended in that window comes // snapshot copy, up to two minutes behind the game box, so it is never
// back from gogobee as rejected_not_running, which is the honest answer. // authoritative and is only allowed the last word where being two minutes late
// cannot make it wrong.
characterName := "" characterName := ""
entry, haveEntry, err := storage.RosterEntryByToken(token) entry, haveEntry, err := storage.RosterEntryByToken(token)
if err != nil { if err != nil {
@@ -125,20 +126,25 @@ func (s *Server) handleAdvOrder(w http.ResponseWriter, r *http.Request) {
if haveEntry { if haveEntry {
characterName = entry.Name characterName = entry.Name
} }
switch req.Action { // No pre-check on extract, deliberately, and it is the same call abandon and
case storage.AdvActionExtract: // leave make in resolveAdvOrderParams: the mark's status is up to two minutes
if haveEntry && entry.Status != "expedition" { // stale here and a Matrix departure can outrun the roster push, so "reads idle"
writeAdvOrderError(w, http.StatusConflict, "you're not on an expedition") // would refuse a run gogobee would happily have ended. The cost accepted is
return // that a genuine mistake comes back as rejected_not_running rather than as an
} // instant refusal, which is the honest answer anyway.
case storage.AdvActionSiegeJoin: if req.Action == storage.AdvActionSiegeJoin {
snap, known, err := storage.LoadSiege() // This one stays, because it is not a personal status: whether a boss is
// camped outside town is a town-wide fact on a day-or-longer clock, so a
// two-minute-old copy is almost never wrong about it. Note the known/active
// split — no snapshot at all must queue the order (a fresh deploy must not
// have a dead button); only a snapshot that positively says active=0 refuses.
active, known, err := storage.SiegeIsCamped()
if err != nil { if err != nil {
slog.Error("orders: siege lookup", "err", err) slog.Error("orders: siege lookup", "err", err)
writeAdvOrderError(w, http.StatusInternalServerError, "internal error") writeAdvOrderError(w, http.StatusInternalServerError, "internal error")
return return
} }
if known && !snap.Active { if known && !active {
writeAdvOrderError(w, http.StatusConflict, "no Siege is camped outside town") writeAdvOrderError(w, http.StatusConflict, "no Siege is camped outside town")
return return
} }
@@ -207,7 +213,11 @@ func resolveAdvOrderParams(owner, token string, req advOrderReq) (*storage.AdvOr
return nil, "pick somewhere to go first" return nil, "pick somewhere to go first"
} }
if len(detail.Zones) == 0 { if len(detail.Zones) == 0 {
return nil, "you're already out there" // An empty offer list usually means they are already out there, but Pete
// cannot tell that from a game box too old to push offers at all, so say
// only what was actually seen. Unreachable from the page either way — with
// no offers the picker doesn't render — so this is a hand-crafted request.
return nil, "nowhere is on offer for you right now"
} }
for _, z := range detail.Zones { for _, z := range detail.Zones {
if z.ID != req.Zone { if z.ID != req.Zone {
+16 -11
View File
@@ -108,15 +108,18 @@ func TestOnlyOneOutstandingOrderPerVerb(t *testing.T) {
} }
} }
// TestActionPreChecksAreCourtesyOnly pins both halves of a deliberate asymmetry. // TestOnlyTownWideFactsArePreChecked. Pete's copy of the board is up to two
// Pete refuses what its own snapshot says is impossible — but the snapshot is up // minutes behind the game box, so what it may refuse locally turns on whether
// to two minutes old, so the refusal must be cheap and local (a 409 the button // being two minutes late could make the answer wrong. A personal status can:
// shows immediately), never a queued order gogobee has to answer. // somebody who set out over Matrix still reads as idle here, and refusing their
func TestActionPreChecksAreCourtesyOnly(t *testing.T) { // extract would deny a run gogobee would have ended. A boss camped outside town
// Idle mark: extract refused up front. // cannot: that is town-wide and runs on a day-or-longer clock.
func TestOnlyTownWideFactsArePreChecked(t *testing.T) {
// Idle mark: extract goes through anyway, and rejected_not_running is the
// answer if the mark really was standing in town.
s := seedActions(t, "holymachina", "idle") s := seedActions(t, "holymachina", "idle")
if w := placeAction(t, s, "holymachina", "extract"); w.Code != 409 { if w := placeAction(t, s, "holymachina", "extract"); w.Code != 200 {
t.Fatalf("extract while idle = %d, want 409", w.Code) t.Fatalf("extract while idle = %d, want it queued (%s)", w.Code, w.Body.String())
} }
// No Siege pushed at all: unknown, not "inactive". Pete has never heard from // No Siege pushed at all: unknown, not "inactive". Pete has never heard from
@@ -328,10 +331,12 @@ func TestExpeditionParamsAreResolvedAgainstTheOwnersOffers(t *testing.T) {
} }
} }
// An empty zone list means "already out there", not "nowhere to go" — gogobee // An empty zone list is a refusal, and the message says only what Pete saw. It
// omits the offers entirely while the adventurer is on an expedition. Refusing // usually means the adventurer is already out — gogobee omits the offers
// entirely while they are down there — but a game box too old to push offers
// sends the same empty list, so the copy claims nothing about which. Refusing
// cheaply here beats a verdict thirty seconds later saying the same thing. // cheaply here beats a verdict thirty seconds later saying the same thing.
func TestNoZoneOffersMeansAlreadyOut(t *testing.T) { func TestNoZoneOffersMeansNothingOnOffer(t *testing.T) {
s := seedOffers(t, "holymachina", "expedition", storage.PlayerDetail{}) s := seedOffers(t, "holymachina", "expedition", storage.PlayerDetail{})
w := placeParams(t, s, "holymachina", advOrderReq{ w := placeParams(t, s, "holymachina", advOrderReq{
Action: storage.AdvActionExpedition, Zone: "goblin_warrens", Loadout: "lean", Action: storage.AdvActionExpedition, Zone: "goblin_warrens", Loadout: "lean",
+25 -9
View File
@@ -36,19 +36,19 @@ func TestOffersToUndoReadsTheViewersOwnSeat(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
token, status string token, status string
haveParty bool partyKnown bool
party []partySeat party []partySeat
self storage.PlayerDetail self storage.PlayerDetail
abandon, leave, cancel bool abandon, leave, cancel bool
}{ }{
{ {
name: "leader of a party is offered the abandon", name: "leader of a party is offered the abandon",
token: "tok-josie", status: "expedition", haveParty: true, party: party, token: "tok-josie", status: "expedition", partyKnown: true, party: party,
abandon: true, abandon: true,
}, },
{ {
name: "member of a party is offered the exit, never the abandon", name: "member of a party is offered the exit, never the abandon",
token: "tok-cam", status: "expedition", haveParty: true, party: party, token: "tok-cam", status: "expedition", partyKnown: true, party: party,
leave: true, leave: true,
}, },
{ {
@@ -56,7 +56,7 @@ func TestOffersToUndoReadsTheViewersOwnSeat(t *testing.T) {
// two seats), so an empty list on a live run means "nobody else", not // two seats), so an empty list on a live run means "nobody else", not
// "we don't know" — and the one body down there is the leader. // "we don't know" — and the one body down there is the leader.
name: "solo run is offered the abandon", name: "solo run is offered the abandon",
token: "tok-josie", status: "expedition", haveParty: true, token: "tok-josie", status: "expedition", partyKnown: true,
abandon: true, abandon: true,
}, },
{ {
@@ -64,19 +64,35 @@ func TestOffersToUndoReadsTheViewersOwnSeat(t *testing.T) {
// snapshot we do not understand, and the safe answer is to offer // snapshot we do not understand, and the safe answer is to offer
// nothing rather than guess which of the two buttons applies. // nothing rather than guess which of the two buttons applies.
name: "a party with no seat for the viewer offers nothing", name: "a party with no seat for the viewer offers nothing",
token: "tok-nobody", status: "expedition", haveParty: true, party: party, token: "tok-nobody", status: "expedition", partyKnown: true, party: party,
}, },
{ {
// The one that came out of running it. An undecodable public sheet // The one that came out of running it. An undecodable public sheet
// gives the same empty slice as a solo run, and treating the two alike // gives the same empty slice as a solo run, and treating the two alike
// offered a party MEMBER the abandon — convincingly, with the rest of // offered a party MEMBER the abandon — convincingly, with the rest of
// the page looking fine. haveParty is what tells them apart. // the page looking fine.
name: "a run whose sheet did not decode offers nothing", name: "a run whose sheet did not decode offers nothing",
token: "tok-cam", status: "expedition", haveParty: false, token: "tok-cam", status: "expedition", partyKnown: false,
},
{
// The same empty slice again, this time from a gogobee too old to push
// seats at all. It decodes fine, so only the sender's own flag tells it
// apart from the solo case two rows up.
name: "an empty party from a sender that never pushes seats offers nothing",
token: "tok-cam", status: "expedition", partyKnown: false, party: nil,
},
{
// The asymmetry: the seat list is self-evidencing, so it keeps working
// against a sender whose capability we cannot confirm. A seat saying
// "member" is not a guess, and refusing the exit here would strand
// somebody in a party for the length of the rollout.
name: "a seated member is offered the exit even without the flag",
token: "tok-cam", status: "expedition", partyKnown: false, party: party,
leave: true,
}, },
{ {
name: "standing in town with nothing open offers nothing", name: "standing in town with nothing open offers nothing",
token: "tok-josie", status: "idle", haveParty: true, party: nil, token: "tok-josie", status: "idle", partyKnown: true, party: nil,
}, },
{ {
// The case the roster status cannot see: an extracted expedition is // The case the roster status cannot see: an extracted expedition is
@@ -101,7 +117,7 @@ func TestOffersToUndoReadsTheViewersOwnSeat(t *testing.T) {
} }
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
abandon, leave, cancel := offersToUndo(tc.token, tc.status, tc.haveParty, tc.party, tc.self) abandon, leave, cancel := offersToUndo(tc.token, tc.status, tc.partyKnown, tc.party, tc.self)
if abandon != tc.abandon || leave != tc.leave || cancel != tc.cancel { if abandon != tc.abandon || leave != tc.leave || cancel != tc.cancel {
t.Fatalf("offers = abandon:%v leave:%v cancel:%v, want abandon:%v leave:%v cancel:%v", t.Fatalf("offers = abandon:%v leave:%v cancel:%v, want abandon:%v leave:%v cancel:%v",
abandon, leave, cancel, tc.abandon, tc.leave, tc.cancel) abandon, leave, cancel, tc.abandon, tc.leave, tc.cancel)
+31
View File
@@ -236,6 +236,37 @@ func TestSiegeHistoryRendersEndedBar(t *testing.T) {
} }
} }
// TestSiegeHistoryCollisionDoesNotFreezeTheWarRoom. boss_id is the history's
// primary key and it is not settled whether gogobee means the siege instance or
// the boss type by it, so two rows can arrive sharing one. Under a bare INSERT
// that failed the transaction carrying the live boss and the muster too, and the
// war room stopped moving on the last good snapshot with nothing to say why. The
// ingest has to degrade to a lost history row instead.
func TestSiegeHistoryCollisionDoesNotFreezeTheWarRoom(t *testing.T) {
s, _ := newAdvServer(t, "tok")
now := time.Now().Unix()
push := liveSiege(now, 650)
push.Siege.History = []storage.SiegePast{
{BossID: 3, BossName: "The Ashen Wyrm", Tier: 5, Outcome: "survived",
HPRemaining: 300, HPMax: 1200, Defenders: 3, EndedAt: now - 86400},
{BossID: 3, BossName: "The Ashen Wyrm", Tier: 5, Outcome: "defeated",
HPRemaining: 0, HPMax: 1200, Defenders: 6, EndedAt: now - 30*86400},
}
if w := postSiege(t, s, "tok", push); w.Code != 200 {
t.Fatalf("push with a duplicated boss_id = %d, want 200 (%s)", w.Code, w.Body.String())
}
v := s.siege()
if !v.Active || v.HPCurrent != 650 {
t.Fatalf("war room = active %v at %d hp, want the pushed live boss — the collision took the whole push down",
v.Active, v.HPCurrent)
}
if len(v.History) != 1 {
t.Errorf("history = %d rows, want 1 (the last of the colliding pair)", len(v.History))
}
}
// TestSiegeAPIFeedsTheBar. The bar only moves because this endpoint answers, so // TestSiegeAPIFeedsTheBar. The bar only moves because this endpoint answers, so
// the field names it emits are load-bearing: the page's JS reads hp_percent to // the field names it emits are load-bearing: the page's JS reads hp_percent to
// set the width and active to decide whether to keep polling at all. // set the width and active to decide whether to keep polling at all.
+29 -16
View File
@@ -38,6 +38,13 @@ type whoDetail struct {
Map *whoMap `json:"map"` Map *whoMap `json:"map"`
// Party is who else is down there, leader first. Absent on a solo run. // Party is who else is down there, leader first. Absent on a solo run.
Party []partySeat `json:"party"` Party []partySeat `json:"party"`
// PartyKnown says the sheet was built by a gogobee that knows about party
// seats, which an absent Party cannot: nil means "solo" and "this sender never
// pushes seats" identically, and those two want opposite buttons. It is a
// capability flag about the SENDER, set unconditionally by any gogobee new
// enough — including on a solo run, including when the seat list is omitted —
// so it must never be read as a fact about the character.
PartyKnown bool `json:"party_known"`
} }
// partySeat is one body on a shared expedition as gogobee described it. Kind is // partySeat is one body on a shared expedition as gogobee described it. Kind is
@@ -120,28 +127,34 @@ const rosterStatusExpedition = "expedition"
// its refusal is the real answer. What this buys is a page that does not put // its refusal is the real answer. What this buys is a page that does not put
// "Leave the party" in front of somebody standing in town. // "Leave the party" in front of somebody standing in town.
// //
// Nothing new crosses the wire for it. Leadership is already legible in the party // Leadership is legible in the party seats gogobee pushes (W7) and the sitter's
// seats gogobee pushes (W7), and the sitter's standing is already in the babysit // standing is in the babysit offer (W5b), so the facts the buttons need are on
// offer (W5b) — so the two facts the buttons need were both already here. // the wire already — except for one, which nil could not express. partyKnown is
// haveParty says whether the public detail blob decoded at all, and it is // the sender's "I know about party seats" flag, and it gates the empty-list
// load-bearing rather than defensive. An empty seat list means "solo" ONLY when // branch alone. An empty list means "solo, so this player is the leader" ONLY
// we have actually read the sheet; a blob that did not decode — a gogobee too old // from a sender that would have listed seats if there were any; from a blob that
// to push seats, a truncated column, a shape change — produces the same empty // did not decode, or from a gogobee too old to push seats at all, the same empty
// slice, and treating that as solo would offer a party MEMBER the button that // slice would offer a party MEMBER the button that throws away everyone's day.
// throws away everyone's day. Found by getting a fixture wrong: the page did // Found by getting a fixture wrong: the page did exactly that, silently and
// exactly that, silently and convincingly. // convincingly.
func offersToUndo(token, status string, haveParty bool, party []partySeat, self storage.PlayerDetail) (abandon, leave, cancelSitter bool) { //
// The asymmetry is deliberate: the len(party) > 0 branch reads the viewer's own
// seat and is self-evidencing — a seat that says "member" cannot be mistaken for
// leadership — so it needs no flag and keeps working against every sender.
func offersToUndo(token, status string, partyKnown bool, party []partySeat, self storage.PlayerDetail) (abandon, leave, cancelSitter bool) {
// The sitter first, because it is the only one of the three whose fact does // The sitter first, because it is the only one of the three whose fact does
// not go stale: an engagement is a property of the character, not of where // not go stale: an engagement is a property of the character, not of where
// they are standing, so a two-minute-old snapshot is still right about it. // they are standing, so a two-minute-old snapshot is still right about it.
cancelSitter = self.Babysit != nil && self.Babysit.Active cancelSitter = self.Babysit != nil && self.Babysit.Active
if status == rosterStatusExpedition && haveParty { if status == rosterStatusExpedition {
if len(party) == 0 { if len(party) == 0 {
// A SOLO run publishes no party at all — partySeatViews returns nil // A SOLO run publishes no party at all — partySeatViews returns nil
// below two seats — so an empty list here is not "we don't know", it is // below two seats — so from a sender that knows about seats an empty list
// "there is nobody else", which makes this player the leader. // is not "we don't know", it is "there is nobody else", which makes this
abandon = true // player the leader. Without the flag it is exactly "we don't know", and
// the button stays off.
abandon = partyKnown
} else { } else {
// With a party, offer strictly on the viewer's own seat, and offer // With a party, offer strictly on the viewer's own seat, and offer
// nothing at all if we cannot find it. Guessing in that case would mean // nothing at all if we cannot find it. Guessing in that case would mean
@@ -402,7 +415,7 @@ func (s *Server) handleAdventureWho(w http.ResponseWriter, r *http.Request) {
} }
} }
page.CanAbandon, page.CanLeave, page.CanCancelSitter = page.CanAbandon, page.CanLeave, page.CanCancelSitter =
offersToUndo(token, entry.Status, page.HasDetail, page.Detail.Party, self) offersToUndo(token, entry.Status, page.HasDetail && page.Detail.PartyKnown, page.Detail.Party, self)
} }
} }
} }