fix(ecleanse.lic): v2.3.1 bound search attempts, validate room between searches, stop duplicate events

[mrhoribu]

Summary

Hive apparatus and ground trap handling could search 30+ times over ~2.5 minutes with combat and support scripts paused, long after the trap had already resolved. The attempt cap of 10 was a symptom, not the cause: the loop's early-exit conditions were unreachable, and duplicate events multiplied whatever the cap allowed.

Root cause

1. The search loop's break conditions were dead code.

lines = Util.get_command("search", /d100: /)
...
next if lines.any? { |l| l =~ /Failure!/ }
break if lines.any? { |l| l =~ /Success|You don't find anything of interest here/ }

/d100: / is passed to Lich::Util.issue_command as start_pattern. issue_command discards every line before that pattern and returns [] when it times out (until (line = get) =~ start_pattern; end — lich-5 lib/util/util.rb). Search outcomes that produce no roll — You don't find anything of interest here., You can't see well enough to search around! — therefore came back as an empty array after burning the full 5s timeout. Both the Failure! retry and the Success/nothing-found break were unreachable in exactly the cases they were written for, leaving search_count >= 10 as the only exit.

2. The duplicate-event guard never worked.

Ecleanse.data.event_stack << :hive_traps_ground && !Ecleanse.data.event_stack.include?(:hive_traps_ground)

<< binds tighter than &&, so this parses as (stack << sym) && (!stack.include?(sym)). The push is unconditional and the boolean is discarded.

Observed in the wild (Kresh Warrens, 2026-08-05):

• 11:14:55 — three matching ground-trap lines arrived in the same second, queuing three runs. 32 searches spanning 11:14:55–11:17:24, with bigshot, volnrestore and 506-trog2 paused the entire time (scripts_resume only fires once event_stack drains). The first search had already cleared the trap: As you begin sifting through the churning earth, the creature within retreats deeper into the ground and the shifting silt goes still.
• 10:15:15 — 11 searches; blinded mid-loop, then knocked unconscious, still searching.

Changes

• Action.hive_search — single helper shared by both trap types, returning :found / :clear / :blind / :moved / :muckled / :exhausted / :timeout so each caller decides what to do next. Removes the duplicated loop body.
• Room validated before every search, not just on entry. Returns :moved the moment we're somewhere else.
• Break on incapacitation — dead? || muckled?. Status.muckled? covers webbed / dead / stunned / bound / sleeping, and notably not prone, so a knockdown from the trap itself won't abort the disarm.
• Attempt caps of 3 for both SEARCH and DISARM, plus a 20s wall-clock deadline. The deadline is load-bearing: get_command's internal ...wait N seconds retry re-issues the command without touching the caller's counter, which is how a cap of 10 became 12+ searches in the log.
• hive_search_result / hive_disarm_result — start_pattern unions covering every observed outcome, so a non-rolled result exits on attempt one instead of timing out silently.
• Tuning values are module methods, not constants, so re-running the script neither emits already initialized constant nor pins the previous value until Lich restarts.
• Action.hive_trap_room? — requires a non-nil marker on both sides, closing the unmapped-room case where nil == Room.current read as a match, and compares room ids. Room overrides neither == nor eql?, so the old comparison was object identity, which fails across a Map.load that rebuilds every Room instance in @@list.
• Event push precedence fixed to unless ... include? for all six guarded pushes: :hive_traps_apparatus, :hive_traps_ground, :sanctum_recover, :use_vat, :itchy_curse, :remove_web_bound. The four unguarded :recover pushes are left alone — they dedupe per-noun via recover_stuff and a symbol-level guard would drop a second disarmed weapon.
• Marker lifecycle — hive_trap_room cleared once the trap resolves or we give up, so any queued duplicate no-ops; preserved on :moved / :muckled so the trap's next announcement can retry.

Behavior

Scenario	Before	After
Trap cleared on first search	10 searches	1
Nothing to find	10 searches (~50s)	1
Rolled Failure!	up to 10	up to 3
Blinded mid-loop	kept searching	breaks immediately
Moved out of the room	kept searching	breaks immediately
Stunned / webbed / unconscious	kept searching	breaks immediately
N duplicate trigger lines	N × 10 searches	1 run

Verification

• ruby -c clean.
• Search state machine exercised against scripted get_command outcomes: success first try → 1 search; nothing-found → 1; all failures → 3 and :exhausted; empty/timeout → 3; blinded → :blind at 1; stale marker → :moved with 0 searches; nil marker or nil Room.current → :moved; muckled → :muckled with 0 searches; moved after the first search → :moved; same room id via a fresh Room instance → :found.
• Marker lifecycle: cleared on resolution so a queued duplicate performs 0 searches, preserved on a muckled bail so the next trap announcement can retry.

Follow-up (not in this PR)

The search and disarm loops still share their guard scaffolding (deadline, attempt cap, room, muckled). Collapsing them into one parameterized helper needs an ordered pattern-to-symbol table, since search distinguishes :blind / :clear / :found and retries on Failure! while disarm has a single terminal pattern. Roughly 12 lines saved for a layer of indirection — worth doing only if a third trap type shows up.

[coderabbitai]

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: bounded search attempts, room validation, and duplicate-event prevention.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/ecleanse.lic (1)

1550-1566: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Apply the same duplicate-event fix to the remaining push sites.

Lines 1559 and 1562 now use the correct unless ... include? guard. Lines 1550, 1552, 1564, and 1566 still use Ecleanse.data.event_stack << :sym && !Ecleanse.data.event_stack.include?(:sym). In that expression << binds first, so the symbol is always pushed and the && result is discarded. These four events can queue duplicates.

🐛 Proposed fix for the remaining sites

-          Ecleanse.data.event_stack << :sanctum_recover && !Ecleanse.data.event_stack.include?(:sanctum_recover)
+          Ecleanse.data.event_stack << :sanctum_recover unless Ecleanse.data.event_stack.include?(:sanctum_recover)
         elsif server =~ /The flesh around the wound feels hot and cold at the same time, heavy with infection./
-          Ecleanse.data.event_stack << :use_vat && !Ecleanse.data.event_stack.include?(:use_vat)
+          Ecleanse.data.event_stack << :use_vat unless Ecleanse.data.event_stack.include?(:use_vat)

-          Ecleanse.data.event_stack << :itchy_curse && !Ecleanse.data.event_stack.include?(:itchy_curse)
+          Ecleanse.data.event_stack << :itchy_curse unless Ecleanse.data.event_stack.include?(:itchy_curse)
         elsif server =~ /^An unseen force entangles you, restricting your movement!\r?\n?$/
-          Ecleanse.data.event_stack << :remove_web_bound && !Ecleanse.data.event_stack.include?(:remove_web_bound)
+          Ecleanse.data.event_stack << :remove_web_bound unless Ecleanse.data.event_stack.include?(:remove_web_bound)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ecleanse.lic` around lines 1550 - 1566, Update the remaining event
pushes in the server-message handling branches for :sanctum_recover, :use_vat,
:itchy_curse, and :remove_web_bound to guard insertion with an unless
event_stack.include? check. Preserve the existing event symbols and ensure each
is appended only when not already present, matching the corrected hive-trap and
recovery-event patterns.

🧹 Nitpick comments (2)

scripts/ecleanse.lic (2)

925-928: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider comparing room identity explicitly.

hive_trap_room? relies on Room instance equality. Lich caches Room objects, so this works for mapped rooms. For robustness, compare the room ids when both ids are present, and fall back to object equality only when an id is nil. This is optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ecleanse.lic` around lines 925 - 928, Update the hive_trap_room?
method to compare room ids when both the stored hive trap room and Room.current
have non-nil ids, falling back to object equality only when either id is nil.
Preserve the existing false result when no hive trap room is configured.

---

939-958: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the bounded disarm loop into a helper.

The disarm loop repeats the deadline, attempt counter, room check, and muckled check from hive_search. Extract a shared bounded-command helper that takes the command, the result pattern, the success pattern, and the attempt limit. Both searches and disarms can then return the same status symbols. This is optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ecleanse.lic` around lines 939 - 958, Optionally extract the repeated
bounded command logic from the hive search and disarm flows into a shared
helper, reusing the existing deadline, attempt-limit, room, and muckled checks.
Have the helper accept the command, result pattern, success pattern, and attempt
limit, and update both callers to return the same status symbols while
preserving their current behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/ecleanse.lic`:
- Around line 902-921: Update hive_search so the result assigned to lines is
always an array by converting Util.get_command("search", hive_search_result)
with to_a. Preserve the existing response matching and retry behavior, allowing
nil results to fall through without raising.

---

Outside diff comments:
In `@scripts/ecleanse.lic`:
- Around line 1550-1566: Update the remaining event pushes in the server-message
handling branches for :sanctum_recover, :use_vat, :itchy_curse, and
:remove_web_bound to guard insertion with an unless event_stack.include? check.
Preserve the existing event symbols and ensure each is appended only when not
already present, matching the corrected hive-trap and recovery-event patterns.

---

Nitpick comments:
In `@scripts/ecleanse.lic`:
- Around line 925-928: Update the hive_trap_room? method to compare room ids
when both the stored hive trap room and Room.current have non-nil ids, falling
back to object equality only when either id is nil. Preserve the existing false
result when no hive trap room is configured.
- Around line 939-958: Optionally extract the repeated bounded command logic
from the hive search and disarm flows into a shared helper, reusing the existing
deadline, attempt-limit, room, and muckled checks. Have the helper accept the
command, result pattern, success pattern, and attempt limit, and update both
callers to return the same status symbols while preserving their current
behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 29a6bd74-1821-4579-b1ab-00e8a10d42c7

📥 Commits

Reviewing files that changed from the base of the PR and between 74340cb and 369bf95.

📒 Files selected for processing (1)

• scripts/ecleanse.lic