diff --git a/internal/peteclient/client.go b/internal/peteclient/client.go index 73b15c2..144e2e0 100644 --- a/internal/peteclient/client.go +++ b/internal/peteclient/client.go @@ -1123,6 +1123,12 @@ const ( AdvOrderExpedition = "expedition_start" AdvOrderResume = "expedition_resume" AdvOrderBabysit = "babysit" + // The three doors the web verbs' own refusal text used to name without + // offering: `!expedition abandon`, `!expedition leave`, `!adventure babysit + // cancel`. None of them takes an argument and none of them spends money. + AdvOrderAbandon = "expedition_abandon" + AdvOrderLeave = "expedition_leave" + AdvOrderBabysitCancel = "babysit_cancel" ) // PendingOrders asks Pete for web actions waiting on us. A Pete predating the diff --git a/internal/plugin/adventure_babysit.go b/internal/plugin/adventure_babysit.go index 9084d3d..3cd62bd 100644 --- a/internal/plugin/adventure_babysit.go +++ b/internal/plugin/adventure_babysit.go @@ -278,25 +278,49 @@ func (p *AdventurePlugin) handleBabysitStatus(ctx MessageContext) error { return p.SendDM(ctx.Sender, text) } -func (p *AdventurePlugin) handleBabysitCancel(ctx MessageContext) error { - userMu := p.advUserLock(ctx.Sender) +// babysitCancelOutcome is what dismissing the sitter did. Summary is the Matrix +// block of what they got through while they were here β€” a paragraph of counts, +// which is right under a DM and too much for a one-line web verdict, so the +// caller decides whether to print it. +type babysitCancelOutcome struct { + Summary string + PetName string +} + +// errBabysitNoSitter is the one way cancelling can be refused. There is no +// "already cancelled" race to worry about: the check and the write are both under +// the per-user lock this takes. +var errBabysitNoSitter = errors.New("babysit: no sitter to dismiss") + +// performBabysitCancel is `!adventure babysit cancel` minus the command framing. +// Shared with the web action queue. +// +// This one TAKES the per-user lock, unlike the abandon/leave twins beside it in +// the order path β€” because its Matrix caller does not hold it (handleBabysitCmd +// dispatches straight here, where `!expedition` holds the lock across its whole +// switch). Its web wrapper must therefore NOT take it. The asymmetry is per verb +// and is worth checking against the Matrix caller every time one is added. +// +// No refund, by design: the sitter was already here. +func (p *AdventurePlugin) performBabysitCancel(uid id.UserID) (babysitCancelOutcome, error) { + userMu := p.advUserLock(uid) userMu.Lock() defer userMu.Unlock() - char, err := loadAdvCharacter(ctx.Sender) + char, err := loadAdvCharacter(uid) if err != nil { - return p.SendDM(ctx.Sender, "No adventurer found.") + return babysitCancelOutcome{}, refuseAdv(errBabysitNoCharacter, "No adventurer found.") } if !char.BabysitActive { - return p.SendDM(ctx.Sender, "🍼 There's nothing to cancel. The babysitter isn't here.") + return babysitCancelOutcome{}, refuseAdv(errBabysitNoSitter, "🍼 There's nothing to cancel. The babysitter isn't here.") } logs, err := loadBabysitLogs(char.UserID) if err != nil { slog.Error("babysit: failed to load logs", "user", char.UserID, "err", err) } - summary := renderBabysitSummary(char, logs) + out := babysitCancelOutcome{Summary: renderBabysitSummary(char, logs), PetName: char.PetName} char.BabysitActive = false char.BabysitExpiresAt = nil @@ -308,7 +332,15 @@ func (p *AdventurePlugin) handleBabysitCancel(ctx MessageContext) error { slog.Error("player_meta: babysit cancel dual-write failed", "user", char.UserID, "err", err) } - return p.SendDM(ctx.Sender, "🍼 Service cancelled. No refund. The babysitter was already there.\n\n"+summary) + return out, nil +} + +func (p *AdventurePlugin) handleBabysitCancel(ctx MessageContext) error { + out, err := p.performBabysitCancel(ctx.Sender) + if err != nil { + return p.SendDM(ctx.Sender, err.Error()) + } + return p.SendDM(ctx.Sender, "🍼 Service cancelled. No refund. The babysitter was already there.\n\n"+out.Summary) } // ── Expiry Check ──────────────────────────────────────────────────────────── diff --git a/internal/plugin/dnd_expedition_cmd.go b/internal/plugin/dnd_expedition_cmd.go index 254aec9..239f464 100644 --- a/internal/plugin/dnd_expedition_cmd.go +++ b/internal/plugin/dnd_expedition_cmd.go @@ -844,65 +844,102 @@ func formatLogTimestamp(t time.Time) string { // ── abandon ───────────────────────────────────────────────────────────────── -func (p *AdventurePlugin) expeditionCmdAbandon(ctx MessageContext) error { - exp, isLeader, err := activeExpeditionFor(ctx.Sender) +// Sentinels for the two ways abandoning can be refused, so the web action queue +// can pick a verdict without reading prose. Same contract as the start/resume +// family above: errors.Is classifies, Error() is the sentence Matrix has always +// sent. +var ( + errAbandonNothing = errors.New("expedition abandon: nothing to abandon") + errAbandonNotLeader = errors.New("expedition abandon: only the leader may call it") +) + +// abandonOutcome is what closing the expedition did, for a caller describing it +// somewhere other than a DM. +type abandonOutcome struct { + Zone ZoneDefinition + Day int + Extracted bool // it was already out and standing in town: loot and XP are kept +} + +// performExpeditionAbandon is `!expedition abandon` minus the command framing. +// Shared with the web action queue so closing a run from a phone disbands the +// same roster, retires the same region runs, writes the same log line and tells +// the same party β€” the members hear it from their leader either way, because +// that is a fact about the expedition and not about which door was used. +// +// Like performExpeditionStart this does NOT take the per-user lock: its Matrix +// caller already holds it across the whole `!expedition` switch. applyWebAbandon +// takes it instead. Getting that backwards does not fail loudly β€” it wedges the +// player's lock forever and every later adventure command from them hangs. +func (p *AdventurePlugin) performExpeditionAbandon(uid id.UserID) (abandonOutcome, error) { + exp, isLeader, err := activeExpeditionFor(uid) if err != nil { - return p.SendDM(ctx.Sender, "Couldn't read expedition state: "+err.Error()) + return abandonOutcome{}, err } if exp == nil { // An extracted expedition is still the owner's to close β€” it holds the // roster until the resume window lapses. Without this, a leader who // wanted out had to pay to `!resume` first just to abandon. - if exp, err = getResumableExpedition(ctx.Sender); err != nil { - return p.SendDM(ctx.Sender, "Couldn't read expedition state: "+err.Error()) + if exp, err = getResumableExpedition(uid); err != nil { + return abandonOutcome{}, err } isLeader = exp != nil } if exp == nil { - return p.SendDM(ctx.Sender, "No active expedition to abandon.") + return abandonOutcome{}, refuseAdv(errAbandonNothing, "No active expedition to abandon.") } if !isLeader { // Abandoning throws away everyone's day. A member leaves alone. - return p.SendDM(ctx.Sender, + return abandonOutcome{}, refuseAdv(errAbandonNotLeader, "Only your party leader can abandon the expedition. `!expedition leave` to walk out alone.") } zone, _ := getZone(exp.ZoneID) - extracted := exp.Status == ExpeditionStatusExtracting + out := abandonOutcome{Zone: zone, Day: exp.CurrentDay, Extracted: exp.Status == ExpeditionStatusExtracting} audience := expeditionAudience(exp) // read before abandonExpedition disbands the roster - if err := abandonExpedition(ctx.Sender); err != nil { - return p.SendDM(ctx.Sender, "Couldn't abandon: "+err.Error()) + if err := abandonExpedition(uid); err != nil { + return abandonOutcome{}, err } - markActedToday(ctx.Sender) + markActedToday(uid) _ = retireAllRegionRuns(exp) _ = appendExpeditionLog(exp.ID, exp.CurrentDay, "narrative", "expedition abandoned", "") + // The roster is being disbanded out from under the members; they hear it from + // their leader rather than discovering it the next time a command works again. + for _, member := range audience { + if member == uid { + continue + } + if err := p.SendDM(member, fmt.Sprintf( + "Your leader called off the expedition in **%s** on Day %d. You're free to start a run of your own.", + zone.Display, exp.CurrentDay)); err != nil { + slog.Warn("expedition: abandon DM failed", "user", member, "expedition", exp.ID, "err", err) + } + } + // Emergence seam: see maybeRollPetArrivalOnEmerge. Inside the twin because + // walking out of a dungeon is what rolls it, not saying so in a room. + p.maybeRollPetArrivalOnEmerge(uid) + return out, nil +} + +func (p *AdventurePlugin) expeditionCmdAbandon(ctx MessageContext) error { + out, err := p.performExpeditionAbandon(ctx.Sender) + if err != nil { + var refusal advRefusal + if errors.As(err, &refusal) { + return p.SendDM(ctx.Sender, refusal.Error()) + } + return p.SendDM(ctx.Sender, "Couldn't abandon: "+err.Error()) + } // An extracted party is standing in town, not in the dungeon: their supplies // are already spent and their loot is already banked. Say the true thing. body := fmt.Sprintf( "Expedition in **%s** abandoned on Day %d. Supplies are forfeit. The dungeon remembers.", - zone.Display, exp.CurrentDay) - if extracted { + out.Zone.Display, out.Day) + if out.Extracted { body = fmt.Sprintf( "You let the expedition in **%s** go. Day %d is where it ends β€” loot, XP, and coins are kept. The dungeon remembers.", - zone.Display, exp.CurrentDay) + out.Zone.Display, out.Day) } - // The roster is being disbanded out from under the members; they hear it from - // their leader rather than discovering it the next time a command works again. - for _, uid := range audience { - if uid == ctx.Sender { - continue - } - if err := p.SendDM(uid, fmt.Sprintf( - "Your leader called off the expedition in **%s** on Day %d. You're free to start a run of your own.", - zone.Display, exp.CurrentDay)); err != nil { - slog.Warn("expedition: abandon DM failed", "user", uid, "expedition", exp.ID, "err", err) - } - } - if err := p.SendDM(ctx.Sender, body); err != nil { - return err - } - // Emergence seam: see maybeRollPetArrivalOnEmerge. - p.maybeRollPetArrivalOnEmerge(ctx.Sender) - return nil + return p.SendDM(ctx.Sender, body) } // helper: ensure we don't shadow id.UserID import in test harness. diff --git a/internal/plugin/expedition_party_cmd.go b/internal/plugin/expedition_party_cmd.go index dc7bf03..d6f1768 100644 --- a/internal/plugin/expedition_party_cmd.go +++ b/internal/plugin/expedition_party_cmd.go @@ -280,47 +280,77 @@ func (p *AdventurePlugin) expeditionCmdParty(ctx MessageContext) error { return p.SendDM(ctx.Sender, b.String()) } -// expeditionCmdLeave walks a member out. The leader cannot leave β€” their row is -// the expedition β€” so they are pointed at `!extract`, which ends it for all. -func (p *AdventurePlugin) expeditionCmdLeave(ctx MessageContext) error { +// Sentinels for the two ways walking out can be refused, so the web action queue +// can pick a verdict without reading prose. errLeaveIsLeader is deliberately not +// errAbandonNotLeader inverted-and-reused: they are opposite facts about the same +// person and a verdict that conflated them would tell a leader they weren't one. +var ( + errLeaveNothing = errors.New("expedition leave: no expedition to leave") + errLeaveIsLeader = errors.New("expedition leave: the leader's row is the expedition") +) + +// performExpeditionLeave is `!expedition leave` minus the command framing. +// Shared with the web action queue, so a member walking out from a phone unseats +// the same way and the leader is told either way. +// +// Like performExpeditionAbandon this does NOT take the per-user lock β€” its +// Matrix caller holds it across the whole `!expedition` switch, and applyWebLeave +// takes it instead. See the comment on performExpeditionAbandon for what getting +// that backwards costs. +func (p *AdventurePlugin) performExpeditionLeave(uid id.UserID) error { // Resolve the seat the way the guards that trap them do. seatedExpeditionFor // spans `extracting`, which activeExpeditionFor does not: a leader who // extracts and never resumes would otherwise leave their members seated β€” // refused a new adventure by the guard, and told "no active expedition" by // the very command the guard points them at. The exit has to see every state // the gate sees. It already excludes leaders, so they fall through below. - seated, err := seatedExpeditionFor(ctx.Sender) + seated, err := seatedExpeditionFor(uid) if err != nil { - return p.SendDM(ctx.Sender, "Couldn't read expedition state: "+err.Error()) + return err } if seated != nil { - return p.leaveSeatedParty(ctx, seated) + return p.leaveSeatedParty(uid, seated) } - exp, isLeader, err := activeExpeditionFor(ctx.Sender) + exp, isLeader, err := activeExpeditionFor(uid) if err != nil { - return p.SendDM(ctx.Sender, "Couldn't read expedition state: "+err.Error()) + return err } if exp == nil { - return p.SendDM(ctx.Sender, "No active expedition.") + return refuseAdv(errLeaveNothing, "No active expedition.") } if isLeader { - return p.SendDM(ctx.Sender, + return refuseAdv(errLeaveIsLeader, "You're leading this one β€” `!extract` ends it for everyone, or `!expedition abandon` to walk away from it.") } - return p.leaveSeatedParty(ctx, exp) + return p.leaveSeatedParty(uid, exp) } -// leaveSeatedParty unseats a member and tells both ends. Shared by the two ways -// a member's seat resolves: the `extracting` limbo and the plain active party. -func (p *AdventurePlugin) leaveSeatedParty(ctx MessageContext, exp *Expedition) error { - if err := leaveParty(exp.ID, ctx.Sender); err != nil { +// expeditionCmdLeave walks a member out. The leader cannot leave β€” their row is +// the expedition β€” so they are pointed at `!extract`, which ends it for all. +func (p *AdventurePlugin) expeditionCmdLeave(ctx MessageContext) error { + if err := p.performExpeditionLeave(ctx.Sender); err != nil { + var refusal advRefusal + if errors.As(err, &refusal) { + return p.SendDM(ctx.Sender, refusal.Error()) + } return p.SendDM(ctx.Sender, "Couldn't leave: "+err.Error()) } + return p.SendDM(ctx.Sender, "You turn back for town. Your supplies stay with the party.") +} + +// leaveSeatedParty unseats a member and tells the leader. Shared by the two ways +// a member's seat resolves: the `extracting` limbo and the plain active party. +// The *member's* own confirmation is the caller's, because that is the one line +// that differs between a DM and a web verdict. +func (p *AdventurePlugin) leaveSeatedParty(uid id.UserID, exp *Expedition) error { + if err := leaveParty(exp.ID, uid); err != nil { + return err + } // Supplies stay in the pool. They were spent on the expedition, not lent to // it, and clawing them back would let a member starve the party on their way // out of the door. _ = p.SendDM(id.UserID(exp.UserID), fmt.Sprintf( - "**%s** turned back. Their supplies stay with the party.", p.DisplayName(ctx.Sender))) - return p.SendDM(ctx.Sender, "You turn back for town. Your supplies stay with the party.") + "**%s** turned back. Their supplies stay with the party.", p.DisplayName(uid))) + return nil } diff --git a/internal/plugin/pete_orders.go b/internal/plugin/pete_orders.go index 3c73377..cfda1de 100644 --- a/internal/plugin/pete_orders.go +++ b/internal/plugin/pete_orders.go @@ -129,11 +129,14 @@ func (p *AdventurePlugin) fulfilAdvOrder(ctx context.Context, order peteclient.A // commands running the very same code. Taking it here too would deadlock on a // non-reentrant mutex. // -// The one exception is performExpeditionStart, which cannot take it β€” its Matrix -// caller already holds it across the whole `!expedition` switch β€” so -// applyWebExpeditionStart takes it instead. That asymmetry is written down in +// The exceptions are the three twins whose Matrix caller already holds the lock +// across the whole `!expedition` switch and so cannot take it themselves β€” +// performExpeditionStart, performExpeditionAbandon and performExpeditionLeave. +// Their apply wrappers below take it instead. That asymmetry is written down in // both places because getting it wrong does not fail loudly: it wedges the -// player's lock forever and every later adventure command from them hangs. +// player's lock forever and every later adventure command from them hangs. The +// rule for a new verb is not "web wrappers take the lock" β€” it is "look at what +// the Matrix caller does", and the two babysit verbs go the other way. func (p *AdventurePlugin) applyAdvOrder(owner id.UserID, order peteclient.AdvOrder) (status, detail string, retry bool) { switch order.Action { case peteclient.AdvOrderExtract: @@ -184,6 +187,15 @@ func (p *AdventurePlugin) applyAdvOrder(owner id.UserID, order peteclient.AdvOrd case peteclient.AdvOrderBabysit: return p.applyWebBabysit(owner, order) + case peteclient.AdvOrderAbandon: + return p.applyWebAbandon(owner) + + case peteclient.AdvOrderLeave: + return p.applyWebLeave(owner) + + case peteclient.AdvOrderBabysitCancel: + return p.applyWebBabysitCancel(owner) + default: // Pete validates the action before it ever queues an order, so this is a // contract breach, not a user mistake. Reject permanently rather than spin. @@ -326,6 +338,102 @@ func (p *AdventurePlugin) applyWebBabysit(owner id.UserID, order peteclient.AdvO return "applied", note, false } +// ---- the three verbs that take no arguments and spend nothing ------------------- +// +// Each of these was already named inside a verdict the web shows: "!expedition +// abandon first", "!expedition leave to walk out alone", "cancel early (no +// refund)". A page that tells somebody to go and type a command it could have +// offered them is a page with a hole in it, and these three close it. +// +// None of them touches money, so none of them needs an idempotency key β€” the +// guid ledger in fulfilAdvOrder is the whole guard, and a replay it somehow got +// past would be refused honestly ("nothing to abandon") rather than charging +// anybody twice. + +// applyWebAbandon closes the owner's expedition down for good. +func (p *AdventurePlugin) applyWebAbandon(owner id.UserID) (status, detail string, retry bool) { + // performExpeditionAbandon does NOT take the per-user lock (its Matrix caller + // holds it across the whole `!expedition` switch), so this has to. See the + // note on applyAdvOrder. + userMu := p.advUserLock(owner) + userMu.Lock() + defer userMu.Unlock() + + out, err := p.performExpeditionAbandon(owner) + if err != nil { + var refusal advRefusal + if !errors.As(err, &refusal) { + // A DB fault reading or writing expedition state. Nothing partial is + // left behind that a retry would double up, so leave it pending. + slog.Warn("orders: abandon failed", "user", owner, "err", err) + return "", "", true + } + status := "rejected_unavailable" + switch { + case errors.Is(err, errAbandonNothing): + status = "rejected_not_running" + case errors.Is(err, errAbandonNotLeader): + status = "rejected_not_leader" + } + return status, advOrderPlainText(err.Error()), false + } + // The extracted case keeps loot and XP, so saying "supplies are forfeit" there + // would be a straight lie. Same split the DM makes. + if out.Extracted { + return "applied", fmt.Sprintf( + "You let %s go on day %d. Loot, XP and coins are kept.", out.Zone.Display, out.Day), false + } + return "applied", fmt.Sprintf( + "Expedition in %s abandoned on day %d. Supplies are forfeit.", out.Zone.Display, out.Day), false +} + +// applyWebLeave walks a party member out of somebody else's expedition. +func (p *AdventurePlugin) applyWebLeave(owner id.UserID) (status, detail string, retry bool) { + // Same lock asymmetry as applyWebAbandon. + userMu := p.advUserLock(owner) + userMu.Lock() + defer userMu.Unlock() + + if err := p.performExpeditionLeave(owner); err != nil { + var refusal advRefusal + if !errors.As(err, &refusal) { + slog.Warn("orders: leave failed", "user", owner, "err", err) + return "", "", true + } + status := "rejected_unavailable" + switch { + case errors.Is(err, errLeaveNothing): + status = "rejected_not_running" + case errors.Is(err, errLeaveIsLeader): + status = "rejected_is_leader" + } + return status, advOrderPlainText(err.Error()), false + } + return "applied", "You turn back for town. Your supplies stay with the party.", false +} + +// applyWebBabysitCancel dismisses the pet sitter early. +func (p *AdventurePlugin) applyWebBabysitCancel(owner id.UserID) (status, detail string, retry bool) { + // No lock here, and that is not an oversight: performBabysitCancel takes it + // itself, because ITS Matrix caller does not. The opposite of the two above. + out, err := p.performBabysitCancel(owner) + if err != nil { + status := "rejected_unavailable" + switch { + case errors.Is(err, errBabysitNoSitter): + status = "rejected_nothing_to_cancel" + } + return status, advOrderPlainText(err.Error()), false + } + // The DM prints the sitter's whole record of the stay; a verdict is one line + // under a button, so the web gets the fact and the page keeps its shape. + note := "Sitter dismissed. No refund β€” they were already here." + if out.PetName != "" { + note = fmt.Sprintf("Sitter dismissed. No refund. %s is back in your care.", out.PetName) + } + return "applied", note, false +} + // advOrderPlainText strips the Matrix markdown out of a line reused as a web // verdict. Pete renders the detail as text, so asterisks and backticks would show // up literally. The command hints inside those backticks stay β€” a verdict that diff --git a/internal/plugin/pete_orders_test.go b/internal/plugin/pete_orders_test.go index 371343f..663d5d2 100644 --- a/internal/plugin/pete_orders_test.go +++ b/internal/plugin/pete_orders_test.go @@ -47,7 +47,11 @@ func TestAdvOrderLedgerShortCircuitsAReoffer(t *testing.T) { // A retried refusal never reaches a verdict, so the order sits pending forever // and Pete's strip never stops saying "asked for…". func TestExtractOrderRefusalsAreTerminal(t *testing.T) { - setupZoneRunTestDB(t) + // W9: was setupZoneRunTestDB, which copies data/gogobee.db and t.Skip()s when + // it is missing β€” and that file is deleted after every local run, so this and + // the extraction test below have been green-by-skipping since W5a. Neither + // needs a prod row; startExpedition builds everything they touch. + setupEmptyTestDB(t) uid := id.UserID("@web-extract-none:example.org") defer cleanupExpeditions(uid) p := &AdventurePlugin{} @@ -68,7 +72,7 @@ func TestExtractOrderRefusalsAreTerminal(t *testing.T) { // 'extracting' (a resumable limbo) rather than 'abandoned' β€” plus the day burn // and the log line the DM path writes. func TestExtractOrderIsTheSameExtraction(t *testing.T) { - setupZoneRunTestDB(t) + setupEmptyTestDB(t) uid := id.UserID("@web-extract-live:example.org") defer cleanupExpeditions(uid) p := &AdventurePlugin{} diff --git a/internal/plugin/pete_orders_undo_test.go b/internal/plugin/pete_orders_undo_test.go new file mode 100644 index 0000000..a8f3b51 --- /dev/null +++ b/internal/plugin/pete_orders_undo_test.go @@ -0,0 +1,209 @@ +package plugin + +import ( + "strings" + "testing" + "time" + + "gogobee/internal/peteclient" + "maunium.net/go/mautrix/id" +) + +// W9: the three web verbs that undo something β€” abandon an expedition, walk out +// of somebody else's party, send the sitter home. +// +// The thing worth pinning here is not the verdict text, it is the LOCK. Each of +// these three now has a headless twin shared between a Matrix command and the web +// order path, and the two halves take advUserLock on opposite sides: the two +// expedition twins cannot take it (their Matrix caller holds it across the whole +// `!expedition` switch) and their web wrappers must, while the babysit twin takes +// it itself and its web wrapper must not. +// +// Getting either of those backwards does not fail loudly. advUserLock is a plain +// sync.Mutex, so a second acquire parks the goroutine forever with the deferred +// Unlock never running β€” which wedges every later !adventure / !expedition / +// !zone command from that player, not just the one that deadlocked. That is a +// bug that shipped once already (see TestExpeditionAliasesDoNotWedgeTheUserLock), +// so both directions are tested here. +// +// NOTE: these two lock tests HANG rather than fail on regression. The timeout is +// the assertion. + +// TestUndoCommandsDoNotWedgeTheUserLock is the Matrix half: `!expedition abandon` +// and `!expedition leave` reach their twins with the lock already held, so a twin +// that took it itself would park here. +func TestUndoCommandsDoNotWedgeTheUserLock(t *testing.T) { + setupEmptyTestDB(t) + uid := id.UserID("@w9-cmd-lock:example") + t.Cleanup(func() { cleanupExpeditions(uid) }) + + for _, sub := range []string{"abandon", "leave"} { + done := make(chan struct{}) + go func() { + defer close(done) + p := &AdventurePlugin{euro: &EuroPlugin{}} + _ = p.handleDnDExpeditionCmd(MessageContext{Sender: uid}, sub) + }() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatalf("!expedition %s never returned: its twin re-took advUserLock", sub) + } + } +} + +// TestUndoOrdersDoNotWedgeTheUserLock is the web half, and it checks the harder +// half of the same property: not just that the order returns, but that the lock +// is FREE afterwards. A wrapper that took the lock around a twin that also takes +// it would park inside applyAdvOrder; a wrapper that forgot to release would let +// the order finish and wedge the next command instead, which is the failure that +// would have been missed by only timing the call. +func TestUndoOrdersDoNotWedgeTheUserLock(t *testing.T) { + setupEmptyTestDB(t) + uid := id.UserID("@w9-order-lock:example") + t.Cleanup(func() { cleanupExpeditions(uid) }) + + for _, action := range []string{ + peteclient.AdvOrderAbandon, + peteclient.AdvOrderLeave, + peteclient.AdvOrderBabysitCancel, + } { + done := make(chan struct{}) + go func() { + defer close(done) + p := &AdventurePlugin{euro: &EuroPlugin{}} + p.applyAdvOrder(uid, peteclient.AdvOrder{GUID: "g-" + action, Action: action}) + // The lock must be back. Taking it here is what catches a wrapper that + // returned without unlocking. + mu := p.advUserLock(uid) + mu.Lock() + mu.Unlock() + }() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatalf("order %q never returned or never released advUserLock", action) + } + } +} + +// TestUndoOrderRefusalsAreTerminal: none of the three may come back as a retry. +// A retried refusal never reaches a verdict, so the order sits pending forever +// and the panel never stops saying "asked for…". Each also has to carry prose β€” +// the strip prefers gogobee's own sentence over its canned fallback, and an empty +// detail on a refusal is the one case where the page has nothing to show. +func TestUndoOrderRefusalsAreTerminal(t *testing.T) { + setupEmptyTestDB(t) + uid := id.UserID("@w9-refusals:example") + t.Cleanup(func() { cleanupExpeditions(uid) }) + // A real character with nothing going on. Without one, babysit_cancel refuses + // with "no adventurer" and the sitter branch this is meant to cover is never + // reached β€” the first cut of this test passed the wrong assertion for that + // reason. + if err := createAdvCharacter(uid, "w9refusals"); err != nil { + t.Fatalf("createAdvCharacter: %v", err) + } + p := &AdventurePlugin{euro: &EuroPlugin{}} + + cases := []struct { + action string + want string + }{ + // Nothing to abandon and nothing to leave are the same fact from two + // doors, and both are the plain "you aren't on one" answer. + {peteclient.AdvOrderAbandon, "rejected_not_running"}, + {peteclient.AdvOrderLeave, "rejected_not_running"}, + // No sitter is its own verdict rather than rejected_unavailable: the page + // says something specific about it, and "unavailable" reads as a fault. + {peteclient.AdvOrderBabysitCancel, "rejected_nothing_to_cancel"}, + } + for _, tc := range cases { + status, detail, retry := p.applyAdvOrder(uid, peteclient.AdvOrder{ + GUID: "g-" + tc.action, Action: tc.action, + }) + if retry { + t.Fatalf("%s asked for a retry on a refusal; it must be terminal", tc.action) + } + if status != tc.want { + t.Fatalf("%s status = %q, want %q", tc.action, status, tc.want) + } + if strings.TrimSpace(detail) == "" { + t.Fatalf("%s refused with no prose; the panel would have nothing to say", tc.action) + } + } +} + +// TestUndoVerdictsCarryNoMarkdown: every detail line these file is reused from a +// Matrix sentence, and Pete renders a verdict as text. An asterisk or a backtick +// left in it shows up literally under the button. +// +// The backticks matter more than they look: the leave refusal names `!extract` +// and `!expedition abandon`, which is the correct thing to say (the web now has a +// button for one of them and not the other), but it must not say it in markup. +func TestUndoVerdictsCarryNoMarkdown(t *testing.T) { + setupEmptyTestDB(t) + uid := id.UserID("@w9-markdown:example") + t.Cleanup(func() { cleanupExpeditions(uid) }) + p := &AdventurePlugin{euro: &EuroPlugin{}} + + for _, action := range []string{ + peteclient.AdvOrderAbandon, + peteclient.AdvOrderLeave, + peteclient.AdvOrderBabysitCancel, + } { + _, detail, _ := p.applyAdvOrder(uid, peteclient.AdvOrder{GUID: "g-md-" + action, Action: action}) + if strings.ContainsAny(detail, "*`\n") { + t.Fatalf("%s verdict carries markdown or a newline: %q", action, detail) + } + } +} + +// TestWebAbandonIsTheGamesOwnAbandon: the web verb must run the real path, not a +// lookalike. The proof is the state the row lands in β€” no expedition left at all, +// as opposed to the 'extracting' limbo an extraction leaves behind β€” and that the +// verdict says what became of the supplies, which is the one thing a player who +// clicked the wrong button needs to be told. +func TestWebAbandonIsTheGamesOwnAbandon(t *testing.T) { + // setupEmptyTestDB, NOT setupZoneRunTestDB: the latter copies data/gogobee.db + // and t.Skip()s when it is missing, and that file is deleted after every local + // run β€” so a test written on it is green-by-skipping on any clean checkout. + // See the Decisions note in the plan's progress file; W5a's order tests were + // silently skipping for exactly this reason. + setupEmptyTestDB(t) + uid := id.UserID("@w9-abandon-live:example.org") + t.Cleanup(func() { cleanupExpeditions(uid) }) + if err := createAdvCharacter(uid, "w9abandon"); err != nil { + t.Fatalf("createAdvCharacter: %v", err) + } + p := &AdventurePlugin{euro: &EuroPlugin{}} + + if _, err := startExpedition(uid, ZoneGoblinWarrens, "", ExpeditionSupplies{ + Current: 10, Max: 10, DailyBurn: 1, HarshMod: 1, PacksStandard: 1, + }); err != nil { + t.Fatalf("startExpedition: %v", err) + } + + status, detail, retry := p.applyAdvOrder(uid, peteclient.AdvOrder{ + GUID: "g-abandon", Action: peteclient.AdvOrderAbandon, + }) + if retry || status != "applied" { + t.Fatalf("abandon = %q retry=%v detail=%q, want applied", status, retry, detail) + } + if !strings.Contains(strings.ToLower(detail), "supplies") { + t.Fatalf("verdict %q never says the supplies are gone, which is the difference from pulling out", detail) + } + if exp, _, err := activeExpeditionFor(uid); err != nil { + t.Fatalf("read expedition: %v", err) + } else if exp != nil { + t.Fatalf("expedition survived a web abandon with status %q", exp.Status) + } + + // And the second click, which is what a stale page produces: a terminal + // refusal, never a retry and never a second abandon. + status, _, retry = p.applyAdvOrder(uid, peteclient.AdvOrder{ + GUID: "g-abandon-2", Action: peteclient.AdvOrderAbandon, + }) + if retry || status != "rejected_not_running" { + t.Fatalf("re-abandon = %q retry=%v, want a terminal rejected_not_running", status, retry) + } +}