zones: stop the tools/backtrack path losing rooms and inventory

Review fallout from the locked-doors commit. Five defects, all in the
seams that commit opened:

backtrackFromDeadFork stepped to VisitedNodes[idx-1]. That slice is a
first-entry ordered *set*, not a path stack (see appendVisited), so once
a run has doubled back once the entry before CurrentNode can sit on a
different branch entirely — the party teleports across the map to a room
no edge connects. `!revisit` refuses exactly that move via
adjacentNodes; the autopilot has no business doing what the player is
forbidden from doing. Now routed through backtrackTarget, which also
refuses to fall back into a corridor whose only exit is the sealed fork:
the lock rolls are seeded per (run, edge), so walking back in gets the
same answer every time, forever.

The autopilot spent the player's thieves' tools *before* committing the
move, so an advanceZoneRunNode failure left them charged and then had
the caller's backtrack clear the fork they had just paid to open. The
tools are now reported by autoPickWithTools and only removed once the
party is actually through the door.

`sell all` never learned the new "tool" type and turned a €600 set into
€300 of loot — the same silent deletion Robbie was taught to avoid two
commits ago. It was already doing this to "key" quest tokens, so both
now sit with the special gear.

The shop's tools branch asked "is the reply a substring of Thieves'
Tools", which is true for a bare "s", for "to", and for an empty message
body — and it runs ahead of the consumable list, so a stray keystroke in
the Supplies view bought a set. isThievesToolsReply matches on the
item's own words instead.

Plus two cleanups in the same code: Robbie built his haul gifts by
calling consumableCache(tier, 1) in a loop when it already takes a
count, and the unlock flow read the whole inventory three times per
command.

internal/plugin/adventure_robbie.go

@@ -173,13 +173,9 @@ func (p *AdventurePlugin) robbieVisitPlayer(userID id.UserID, displayName string
 		// frozen legacy CombatLevel — that snapshots at 1–3 once D&D setup
 		// completes, so reading it here would peg every gift at tier 1.
 		tier := robbieGiftTier(arenaDnDLevelOrZero(userID))
-		for range robbieGiftCount(char.RobbieVisitCount, len(takenItems)) {
+		for _, gift := range consumableCache(tier, robbieGiftCount(char.RobbieVisitCount, len(takenItems))) {
-			gifts := consumableCache(tier, 1)
+			if err := addAdvInventoryItem(userID, gift); err == nil {
-			if len(gifts) == 0 {
+				leftGifts = append(leftGifts, gift)
-				break
-			}
-			if err := addAdvInventoryItem(userID, gifts[0]); err == nil {
-				leftGifts = append(leftGifts, gifts[0])
 			}
 		}
 		_ = saveAdvCharacter(char)

internal/plugin/adventure_shop.go

@@ -845,7 +845,13 @@ func (p *AdventurePlugin) advSellAll(userID id.UserID) string {
 	var keptConsumable int
 	var keptMagic int
 	for _, item := range items {
-		if item.Type == "MasterworkGear" || item.Type == "ArenaGear" || item.Type == "card" {
+		// Keys and tools ride along with the special gear here for the same
+		// reason Robbie won't take them: both are bought or found precisely so a
+		// door can be opened *later*, and `sell all` is a bulk-loot verb the
+		// player fires after every haul without reading the list. Turning one
+		// into €300 silently deletes the unlock it was carried for.
+		switch item.Type {
+		case "MasterworkGear", "ArenaGear", "card", "key", thievesToolsItemType:
 			keptSpecial++
 			continue
 		}
@@ -1079,7 +1085,7 @@ func (p *AdventurePlugin) resolveShopSupplyChoice(ctx MessageContext, interactio
 	// Thieves' tools sit on the supplies shelf but are not a ConsumableDef:
 	// the combat engine scans inventory against that table and would happily
 	// spend them mid-fight. They get their own branch and their own item type.
-	if containsFold(thievesToolsName, reply) || containsFold("thieves tools", reply) {
+	if isThievesToolsReply(reply) {
 		return p.buyThievesTools(ctx, interaction)
 	}
 

internal/plugin/dnd_expedition_cmd.go

@@ -907,12 +907,14 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf
 
 	ranked := rankForkOptions(g, run, pf)
 	note := "autopilot took the most promising path"
+	var spendTool int64
 	if len(ranked) == 0 {
-		picked, ok := p.autoPickWithTools(run, pf)
+		picked, toolID, ok := p.autoPickWithTools(run, pf)
 		if !ok {
 			return false
 		}
 		ranked = []pendingChoice{picked}
+		spendTool = toolID
 		note = "autopilot spent " + thievesToolsName + " on the only way forward"
 	}
 	chosen := ranked[0]
@@ -922,6 +924,14 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf
 			"user", run.UserID, "run", run.RunID, "err", err)
 		return false
 	}
+	// The door is behind us, so now the set is actually used up. Billing before
+	// the advance would charge the player for a move that failed — and the
+	// caller's backtrack would then clear the fork the tools just paid for.
+	if spendTool != 0 {
+		if err := removeAdvInventoryItem(spendTool); err != nil {
+			slog.Warn("expedition: autopilot tools spend", "user", run.UserID, "err", err)
+		}
+	}
 	fireGraphRegionTransition(run.UserID, g.Nodes[run.CurrentNode], g.Nodes[chosen.To])
 	if exp != nil {
 		_ = appendExpeditionLog(exp.ID, exp.CurrentDay, "narrative",
@@ -930,31 +940,32 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf
 	return true
 }
 
-// autoPickWithTools spends one set of thieves' tools to open a fork where every
+// autoPickWithTools finds a route a set of thieves' tools could open when every
-// route is locked, so a bad Perception roll can't quietly end an expedition the
+// option on the fork is locked, so a bad Perception roll can't quietly end an
-// player paid days into. It only ever fires when there is no free route left —
+// expedition the player paid days into. It only ever fires when there is no free
-// the player's tools are their own, and the autopilot does not get to burn them
+// route left — the player's tools are their own, and the autopilot does not get
-// for convenience.
+// to burn them for convenience.
-func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (pendingChoice, bool) {
+//
-	owner := id.UserID(run.UserID)
+// It reports the route and the inventory row to spend, but does not spend it:
-	toolID, ok := findThievesTools(owner)
+// the caller charges the player only once the move has actually committed.
+func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (pendingChoice, int64, bool) {
+	toolID, ok := findThievesTools(id.UserID(run.UserID))
 	if !ok {
-		return pendingChoice{}, false
+		return pendingChoice{}, 0, false
 	}
 	for i := range pf.Options {
 		if pf.Options[i].Unlocked || !pickableLock(pf.Options[i].Lock) {
 			continue
 		}
-		if err := removeAdvInventoryItem(toolID); err != nil {
+		// Local copy only — advanceZoneRunNode clears node_choices on success,
-			slog.Warn("expedition: autopilot tools spend", "user", run.UserID, "err", err)
+		// and on failure the fork must stay as locked as the player left it
-			return pendingChoice{}, false
+		// rather than reading "open" for a set nobody paid for.
-		}
+		chosen := pf.Options[i]
-		pf.Options[i].Unlocked = true
+		chosen.Unlocked = true
-		pf.Options[i].Reason = "opened with " + thievesToolsName
+		chosen.Reason = "opened with " + thievesToolsName
-		_ = writePendingFork(run.RunID, *pf)
+		return chosen, toolID, true
-		return pf.Options[i], true
 	}
-	return pendingChoice{}, false
+	return pendingChoice{}, 0, false
 }
 
 // backtrackFromDeadFork walks the run back one room when a fork has no route
@@ -963,13 +974,17 @@ func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (p
 // losing days of progress to a die roll they never saw and could not answer.
 // Backtracking at least returns them to a room with other exits.
 //
-// Returns false at the entry node, where there is nowhere behind to go.
+// Returns false at the entry node, where there is nowhere behind to go, and
+// wherever no fallback room would actually help — see backtrackTarget.
 func (p *AdventurePlugin) backtrackFromDeadFork(exp *Expedition, run *DungeonRun) bool {
-	idx := pathIndexOf(run.VisitedNodes, run.CurrentNode)
+	g, ok := loadZoneGraph(run.ZoneID)
-	if idx <= 0 {
+	if !ok {
+		return false
+	}
+	target, ok := backtrackTarget(g, run)
+	if !ok {
 		return false
 	}
-	target := run.VisitedNodes[idx-1]
 
 	// Clear the fork first: it belongs to the node being left, and both
 	// `!zone advance` and `!zone go` would otherwise resolve a prompt pointing
@@ -989,6 +1004,35 @@ func (p *AdventurePlugin) backtrackFromDeadFork(exp *Expedition, run *DungeonRun
 	return true
 }
 
+// backtrackTarget picks the room a dead fork falls back to: the most recently
+// entered visited node that is genuinely joined to the current one by an edge
+// and that still offers a way on other than the sealed room.
+//
+// Both halves are load-bearing. VisitedNodes is a first-entry ordered *set*, not
+// a path stack (see appendVisited) — after any earlier backtrack the entry
+// before CurrentNode can sit on a completely different branch, so stepping to it
+// blind teleports the party across the map. `!revisit` refuses exactly that move
+// via adjacentNodes, and the autopilot has no business doing what the player is
+// forbidden from doing. The second half stops the other failure: falling back
+// into a corridor whose only exit is the fork we just fled from just walks
+// straight back in — and the lock rolls are seeded per (run, edge), so the
+// result is identical every time. That is an infinite loop, not a recovery.
+func backtrackTarget(g ZoneGraph, run *DungeonRun) (string, bool) {
+	adj := adjacentNodes(g, run.CurrentNode)
+	for i := pathIndexOf(run.VisitedNodes, run.CurrentNode) - 1; i >= 0; i-- {
+		n := run.VisitedNodes[i]
+		if !adj[n] {
+			continue
+		}
+		for _, e := range g.outgoingEdges(n) {
+			if e.To != run.CurrentNode {
+				return n, true
+			}
+		}
+	}
+	return "", false
+}
+
 func (p *AdventurePlugin) runAutopilotWalk(ctx MessageContext, maxRooms int, compact, inlineBossCombat bool) autopilotWalkResult {
 	exp, err := getActiveExpedition(ctx.Sender)
 	if err != nil {

internal/plugin/zone_backtrack_test.go

@@ -0,0 +1,60 @@
+package plugin
+
+import "testing"
+
+// backtrackGraph — a fork at z.fork with two branches, plus a dead-end spur
+// hanging off z.b whose only exit is back into z.b.
+//
+//	z.entry → z.fork → z.a
+//	                 → z.b → z.spur
+func backtrackGraph() ZoneGraph {
+	return ZoneGraph{
+		Nodes: map[string]ZoneNode{},
+		Edges: map[string][]ZoneEdge{
+			"z.entry": {{From: "z.entry", To: "z.fork"}},
+			"z.fork":  {{From: "z.fork", To: "z.a"}, {From: "z.fork", To: "z.b"}},
+			"z.b":     {{From: "z.b", To: "z.spur"}},
+		},
+	}
+}
+
+// TestBacktrackTargetRequiresAdjacency is the regression guard for the bug this
+// fixes: VisitedNodes is a first-entry ordered set, not a path stack, so the
+// entry before CurrentNode can sit on a branch the party never walked from
+// here. Stepping to it blind teleports them across the map.
+func TestBacktrackTargetRequiresAdjacency(t *testing.T) {
+	// Walked entry → fork → a, doubled back, then took fork → b. The visited
+	// set is [entry, fork, a, b]; z.a is *not* joined to z.b.
+	run := &DungeonRun{
+		CurrentNode:  "z.b",
+		VisitedNodes: []string{"z.entry", "z.fork", "z.a", "z.b"},
+	}
+	got, ok := backtrackTarget(backtrackGraph(), run)
+	if !ok {
+		t.Fatal("should fall back to the fork")
+	}
+	if got != "z.fork" {
+		t.Errorf("backtrack target = %s, want z.fork (z.a shares no edge with z.b)", got)
+	}
+}
+
+// TestBacktrackTargetSkipsOneWayCorridor — backing into a room whose only exit
+// is the sealed fork walks straight back in, and the lock rolls are seeded per
+// (run, edge), so it would do it forever. Refuse instead.
+func TestBacktrackTargetSkipsOneWayCorridor(t *testing.T) {
+	run := &DungeonRun{
+		CurrentNode:  "z.spur",
+		VisitedNodes: []string{"z.entry", "z.fork", "z.b", "z.spur"},
+	}
+	if got, ok := backtrackTarget(backtrackGraph(), run); ok {
+		t.Errorf("backtracked to %s; z.b only leads back to z.spur, so this loops", got)
+	}
+}
+
+// TestBacktrackTargetAtEntry — nowhere behind the entry node.
+func TestBacktrackTargetAtEntry(t *testing.T) {
+	run := &DungeonRun{CurrentNode: "z.entry", VisitedNodes: []string{"z.entry"}}
+	if _, ok := backtrackTarget(backtrackGraph(), run); ok {
+		t.Error("entry node has nowhere to back out to")
+	}
+}

internal/plugin/zone_graph_unlock.go

@@ -41,35 +41,48 @@ func pickableLock(kind string) bool {
 	return kind == string(LockPerception) || kind == string(LockStatCheck)
 }
 
+// isThievesToolsReply matches a shop reply against the tools.
+//
+// Deliberately not "is the reply a substring of the name": that predicate is
+// true for a bare "s", for "to", and for an empty message body — and this branch
+// sits ahead of the consumable list, so a stray keystroke in the Supplies view
+// would silently bill the player €600. Match on the item's own words instead.
+func isThievesToolsReply(reply string) bool {
+	return containsFold(reply, "thieves") || containsFold(reply, "thief") ||
+		containsFold(reply, "tools")
+}
+
+// thievesToolsHeld returns the inventory row IDs of every set the player is
+// carrying. One read answers both "have they got any" and "how many are left",
+// which the unlock flow would otherwise ask the inventory table three times.
+func thievesToolsHeld(userID id.UserID) []int64 {
+	inv, err := loadAdvInventory(userID)
+	if err != nil {
+		return nil
+	}
+	var ids []int64
+	for _, it := range inv {
+		if strings.EqualFold(it.Name, thievesToolsName) {
+			ids = append(ids, it.ID)
+		}
+	}
+	return ids
+}
+
 // findThievesTools returns the inventory row ID of one set of tools, or ok=false
 // if the player is carrying none.
 func findThievesTools(userID id.UserID) (int64, bool) {
-	inv, err := loadAdvInventory(userID)
+	held := thievesToolsHeld(userID)
-	if err != nil {
+	if len(held) == 0 {
 		return 0, false
 	}
-	for _, it := range inv {
+	return held[0], true
-		if strings.EqualFold(it.Name, thievesToolsName) {
-			return it.ID, true
-		}
-	}
-	return 0, false
 }
 
 // countThievesTools reports how many sets the player carries, for the "N left"
 // line after a use.
 func countThievesTools(userID id.UserID) int {
-	inv, err := loadAdvInventory(userID)
+	return len(thievesToolsHeld(userID))
-	if err != nil {
-		return 0
-	}
-	n := 0
-	for _, it := range inv {
-		if strings.EqualFold(it.Name, thievesToolsName) {
-			n++
-		}
-	}
-	return n
 }
 
 // zoneCmdUnlock handles `!zone unlock <n>`: spend one set of thieves' tools to
@@ -99,12 +112,14 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error {
 		return p.SendDM(ctx.Sender, "No fork pending. Use "+continueHint(ctx.Sender))
 	}
 
+	held := thievesToolsHeld(ctx.Sender)
+
 	rest = strings.TrimSpace(rest)
 	if rest == "" {
 		zone := zoneOrFallback(run.ZoneID)
 		return p.SendDM(ctx.Sender, "**Which one?**\n\n"+renderForkPrompt(zone, *pf)+
 			fmt.Sprintf("\n\n_`!zone unlock <n>` — spends one set of %s. You carry %d._",
-				thievesToolsName, countThievesTools(ctx.Sender)))
+				thievesToolsName, len(held)))
 	}
 	choice := atoiSafe(rest)
 	if choice < 1 || choice > len(pf.Options) {
@@ -121,13 +136,12 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error {
 			"🔒 Tools won't help here. %s\n\n_Thieves' tools answer a failed check, not a locked gate._",
 			lockRefusalFor(chosen)))
 	}
-	toolID, ok := findThievesTools(ctx.Sender)
+	if len(held) == 0 {
-	if !ok {
 		return p.SendDM(ctx.Sender, fmt.Sprintf(
 			"🔒 **%s** needs %s and you're carrying none.\n\nLuigi stocks them under `!shop` → Supplies.",
 			chosen.Label, thievesToolsName))
 	}
-	if rerr := removeAdvInventoryItem(toolID); rerr != nil {
+	if rerr := removeAdvInventoryItem(held[0]); rerr != nil {
 		return p.SendDM(ctx.Sender, "Couldn't spend the tools: "+rerr.Error())
 	}
 
@@ -138,8 +152,8 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error {
 	pf.Options[choice-1].Reason = "opened with " + thievesToolsName
 	_ = writePendingFork(run.RunID, *pf)
 
-	left := countThievesTools(ctx.Sender)
+	header := fmt.Sprintf("🔓 **%s** — picked. _(%s used, %d left)_\n\n",
-	header := fmt.Sprintf("🔓 **%s** — picked. _(%s used, %d left)_\n\n", chosen.Label, thievesToolsName, left)
+		chosen.Label, thievesToolsName, len(held)-1)
 	return p.commitForkChoice(ctx, run, pf.Options[choice-1], header)
 }