feat(healbot): v1.4.0 add undisease/poison feature, improve timeout/hook for transfer#2333
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 25 minutes and 24 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extends healbot with two new healing spell commands (undisease and unpoison), wires them into the trigger system, and fixes concurrency issues by adding timeouts and disabling the problematic room monitoring thread. ChangesHealbot Disease and Poison Healing with Concurrency Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
mrhoribu
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/healbot_core.lic (1)
20-21: 💤 Low valueUnnecessary
Array.newinitialization.Line 20 creates an empty array that is immediately overwritten on line 21. This line can be removed.
Proposed fix
-required_scripts = Array.new required_scripts = ["healbot_actions", "waggle"]🤖 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/healbot_core.lic` around lines 20 - 21, Remove the redundant empty array initialization: delete the line that sets required_scripts = Array.new and keep the subsequent assignment required_scripts = ["healbot_actions", "waggle"]; update any surrounding code/comments if they referenced the initial empty initialization so only the explicit literal assignment remains (look for the variable required_scripts in this file to confirm no other initialization is needed).
🤖 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/healbot_actions.lic`:
- Line 38: The array assignment for healbot_actions is missing spaces after
commas causing RuboCop failures; update the healbot_actions declaration so each
comma is followed by a space (e.g., "eatbread", "givebread", "healdown", ...) in
the healbot_actions variable to satisfy style rules and the pipeline.
- Around line 284-289: Wrap the Timeout::timeout block in a begin/rescue/ensure
so Timeout::Error is rescued and handled (e.g. log or set a failure flag)
instead of propagating, and always remove the downstream hook in an ensure block
to avoid leaving hooks registered (reference Timeout::timeout,
DownstreamHook.add("hb_appraise", action), DownstreamHook.remove("hb_appraise"),
appraisal_done, and the fput "appraise #{character}" call); also fix spacing by
adding a space before the `{` in the Timeout::timeout call (use
Timeout::timeout(3) { ...) and before any `{` that the pipeline flagged.
In `@scripts/healbot_core.lic`:
- Line 14: Update the version lock comment string so it references the correct
file: replace the text "Version locked with healbot_core.lic" in this file's
header with "Version locked with healbot_actions.lic" (i.e., update the literal
comment line in scripts/healbot_core.lic from healbot_core.lic to
healbot_actions.lic).
---
Nitpick comments:
In `@scripts/healbot_core.lic`:
- Around line 20-21: Remove the redundant empty array initialization: delete the
line that sets required_scripts = Array.new and keep the subsequent assignment
required_scripts = ["healbot_actions", "waggle"]; update any surrounding
code/comments if they referenced the initial empty initialization so only the
explicit literal assignment remains (look for the variable required_scripts in
this file to confirm no other initialization is needed).
🪄 Autofix (Beta)
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
Run ID: 0cd8187f-3cf8-41ac-b33c-09c8c0075cd2
📒 Files selected for processing (2)
scripts/healbot_actions.licscripts/healbot_core.lic
| Timeout::timeout(3){ | ||
| DownstreamHook.add("hb_appraise", action) | ||
| fput "appraise #{character}" | ||
| wait_until { appraisal_done } | ||
| } | ||
| DownstreamHook.remove("hb_appraise") |
There was a problem hiding this comment.
Unhandled Timeout::Error will crash the script on timeout.
If the appraise workflow exceeds 3 seconds, Timeout::timeout raises Timeout::Error which propagates uncaught, terminating the script. This defeats the purpose of adding a timeout to "prevent issues when a transfer fails."
Wrap the timeout block in a rescue to handle the timeout gracefully.
Proposed fix
- Timeout::timeout(3){
+ begin
+ Timeout::timeout(3) {
- DownstreamHook.add("hb_appraise", action)
- fput "appraise #{character}"
- wait_until { appraisal_done }
- }
+ DownstreamHook.add("hb_appraise", action)
+ fput "appraise #{character}"
+ wait_until { appraisal_done }
+ }
+ rescue Timeout::Error
+ echo "hb_actions:transfer: Appraise timed out"
+ DownstreamHook.remove("hb_appraise")
+ exit
+ end
DownstreamHook.remove("hb_appraise")Additionally, fix the missing space before { per the pipeline failure:
- Timeout::timeout(3){
+ Timeout::timeout(3) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Timeout::timeout(3){ | |
| DownstreamHook.add("hb_appraise", action) | |
| fput "appraise #{character}" | |
| wait_until { appraisal_done } | |
| } | |
| DownstreamHook.remove("hb_appraise") | |
| begin | |
| Timeout::timeout(3) { | |
| DownstreamHook.add("hb_appraise", action) | |
| fput "appraise #{character}" | |
| wait_until { appraisal_done } | |
| } | |
| rescue Timeout::Error | |
| echo "hb_actions:transfer: Appraise timed out" | |
| DownstreamHook.remove("hb_appraise") | |
| exit | |
| end | |
| DownstreamHook.remove("hb_appraise") |
🧰 Tools
🪛 GitHub Actions: Rubocop / 0_Run Rubocop on Ruby.txt
[error] 284-284: Rubocop: Layout/SpaceBeforeBlockBraces: Space missing to the left of '{'. (Timeout::timeout(3){ ... })
🪛 GitHub Actions: Rubocop / Run Rubocop on Ruby
[error] 284-284: Rubocop: Layout/SpaceBeforeBlockBraces: Space missing to the left of '{'.
🤖 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/healbot_actions.lic` around lines 284 - 289, Wrap the
Timeout::timeout block in a begin/rescue/ensure so Timeout::Error is rescued and
handled (e.g. log or set a failure flag) instead of propagating, and always
remove the downstream hook in an ensure block to avoid leaving hooks registered
(reference Timeout::timeout, DownstreamHook.add("hb_appraise", action),
DownstreamHook.remove("hb_appraise"), appraisal_done, and the fput "appraise
#{character}" call); also fix spacing by adding a space before the `{` in the
Timeout::timeout call (use Timeout::timeout(3) { ...) and before any `{` that
the pipeline flagged.
2 participants
Adds a timeout to the transfer downstream hook to prevent odd circumstances when a transfer fails
Adds Unpoison request
Adds Undisease request
Version locks healbot_actions and healbot_core together
Summary by CodeRabbit
New Features
Bug Fixes