fix(healer): skip redirect for Unity, extend passive heal timeout#7398
Open
MahtraDR wants to merge 1 commit into
Open
fix(healer): skip redirect for Unity, extend passive heal timeout#7398MahtraDR wants to merge 1 commit into
MahtraDR wants to merge 1 commit into
Conversation
…imeout Unity transfers wounds one-for-one to matching body parts, so redirect is unnecessary and the body-part availability gate was incorrectly blocking transfers. Non-Unity transfers still redirect as before. Passive healing timeout no longer fires when Regenerate is active, since Regenerate will heal all wounds given enough time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR updates the dependency library to version 3.1.0, removing obsolete autostart filtering logic while introducing deprecation stubs. In parallel, the Healer script adds per-patient Unity link tracking, refactors wound transfer logic to branch based on link status, adjusts passive healing timeouts, and resets link state on emergency recovery. Comprehensive test coverage validates the new behavior. ChangesDependency Management Cleanup
Healer Unity Link Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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)
⚔️ Resolve merge conflicts
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
healer.lic (1)
777-783:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly persist
unity_linkedon a confirmed Unity success.Line 783 currently treats every non-
not foundresponse as a successful Unity upgrade, butLINK_RESPONSESalso matchesYou strain. That leaves the patient marked as Unity-linked even when the upgrade failed, so later wound transfers skip redirect and take the wrong path.Suggested fix
link_result = DRC.bput("link #{victim} unity", *LINK_RESPONSES) if link_result =~ LINK_NOT_FOUND_PATTERN Lich::Messaging.msg("bold", "Healer: Could not link with #{victim}") remove_patient(victim, reason: :not_found) return end - update_patient(victim, unity_linked: true) + if link_result =~ LINK_SUCCESS_PATTERN + update_patient(victim, unity_linked: true) + else + update_patient(victim, unity_linked: false) + log("Unity link did not stick for #{victim}; falling back to redirect flow") + end🤖 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 `@healer.lic` around lines 777 - 783, The current code sets unity_linked for any DRC.bput response that isn't LINK_NOT_FOUND_PATTERN, which also matches non-success responses like "You strain"; change the logic so unity_linked is only persisted when link_result matches an explicit success pattern (e.g. add or use a LINK_SUCCESS_PATTERN / explicit success regex) after the DRC.bput("link #{victim} unity") call, keep the existing removal on LINK_NOT_FOUND_PATTERN, and avoid calling update_patient(victim, unity_linked: true) for non-success responses (or handle transient failures separately).
🧹 Nitpick comments (1)
spec/healer_spec.rb (1)
957-965: ⚡ Quick winThis spec exits before it ever hits the timeout branch.
The stub flips to
score: 0on the first iteration whereelapsedreaches 30s, so line 605 is never evaluated withelapsed >= PASSIVE_HEAL_MAX_WAIT. That makes this a false positive for the new Regenerate behavior. Keep the patient wounded for one more poll so the test actually crosses the timeout threshold and proves the fallback is suppressed.Suggested fix
allow(DRCH).to receive(:check_health) do call_count += 1 - if call_count <= (Healer::PASSIVE_HEAL_MAX_WAIT / Healer::PASSIVE_HEAL_POLL_INTERVAL) + 1 + if call_count <= (Healer::PASSIVE_HEAL_MAX_WAIT / Healer::PASSIVE_HEAL_POLL_INTERVAL) + 2 health_result(score: 5, wounds: { 2 => [wound(body_part: 'chest', severity: 2)] }) else health_result(score: 0, wounds: {}) end end🤖 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 `@spec/healer_spec.rb` around lines 957 - 965, The test's DRCH.check_health stub returns a healed score too early so the timeout branch in Healer (triggered when elapsed >= Healer::PASSIVE_HEAL_MAX_WAIT) is never exercised; update the stub's condition in the spec (the block that increments call_count and compares it to (Healer::PASSIVE_HEAL_MAX_WAIT / Healer::PASSIVE_HEAL_POLL_INTERVAL) + 1) to keep the patient wounded for one additional poll (e.g., increase the +1 offset by one) so the stub returns score:5 for one more iteration and only flips to score:0 after the tester has crossed the PASSIVE_HEAL_MAX_WAIT threshold, ensuring the timeout branch and Regenerate fallback suppression are actually tested.
🤖 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 `@dependency.lic`:
- Around line 943-952: The migration only clears Settings['autostart'] but
legacy entries in UserVars.autostart_scripts and YAML-loaded autostarts still
get launched; update the migration block to also remove or filter legacy entries
from UserVars.autostart_scripts and any autostarts collection loaded from YAML
(the same place that later iterates/starts autostarts) so obsolete scripts are
not relaunched before the warning runs. Locate the code that reads/merges
UserVars.autostart_scripts and autostarts and either purge entries that match
the obsolete autostart format or set UserVars.autostart_scripts and the merged
autostarts list to an empty/filtered value in the migration so the subsequent
startup loop will skip them.
- Around line 954-970: Change the deprecation stubs to accept arbitrary
arguments so legacy call shapes don't raise ArgumentError: update the method
signatures for autostart and stop_autostart (and optionally dependency_status)
from def autostart(_script_names, _global = true) / def
stop_autostart(_script_names) to accept a splat like def autostart(*_args) and
def stop_autostart(*_args) (and def dependency_status(*_args) if you want it
arity-tolerant too); keep the existing echo messages and ignore the captured
args.
---
Outside diff comments:
In `@healer.lic`:
- Around line 777-783: The current code sets unity_linked for any DRC.bput
response that isn't LINK_NOT_FOUND_PATTERN, which also matches non-success
responses like "You strain"; change the logic so unity_linked is only persisted
when link_result matches an explicit success pattern (e.g. add or use a
LINK_SUCCESS_PATTERN / explicit success regex) after the DRC.bput("link
#{victim} unity") call, keep the existing removal on LINK_NOT_FOUND_PATTERN, and
avoid calling update_patient(victim, unity_linked: true) for non-success
responses (or handle transient failures separately).
---
Nitpick comments:
In `@spec/healer_spec.rb`:
- Around line 957-965: The test's DRCH.check_health stub returns a healed score
too early so the timeout branch in Healer (triggered when elapsed >=
Healer::PASSIVE_HEAL_MAX_WAIT) is never exercised; update the stub's condition
in the spec (the block that increments call_count and compares it to
(Healer::PASSIVE_HEAL_MAX_WAIT / Healer::PASSIVE_HEAL_POLL_INTERVAL) + 1) to
keep the patient wounded for one additional poll (e.g., increase the +1 offset
by one) so the stub returns score:5 for one more iteration and only flips to
score:0 after the tester has crossed the PASSIVE_HEAL_MAX_WAIT threshold,
ensuring the timeout branch and Regenerate fallback suppression are actually
tested.
🪄 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: afcfce7b-421e-449b-9da7-f099d7b2f72a
📒 Files selected for processing (3)
dependency.lichealer.licspec/healer_spec.rb
Comment on lines
+943
to
+952
| # --- One-shot cleanup of orphaned Settings['autostart'] --- | ||
| # The old CORE_AUTOSTART gate merged Settings['autostart'] into | ||
| # UserVars.autostart_scripts on every login, making it impossible | ||
| # to permanently remove entries. Clear the orphaned data once, | ||
| # then this block becomes a no-op on subsequent logins. | ||
| if Settings['autostart'] | ||
| echo("Clearing orphaned Settings['autostart'] (no longer used).") | ||
| Settings['autostart'] = nil | ||
| Settings.save | ||
| end |
There was a problem hiding this comment.
Keep obsolete autostarts from booting.
This migration only clears Settings['autostart']. Line 645 still loads UserVars.autostart_scripts plus YAML autostarts, and Lines 685-687 start them before the later obsolete-script warning runs. Any player carrying a legacy obsolete entry there will now relaunch that script on login instead of being skipped.
Suggested fix
def autostarts
- (UserVars.autostart_scripts.to_a + SetupFiles.new.get_settings.autostarts.to_a).uniq
+ obsolete = defined?(DR_OBSOLETE_SCRIPTS) ? DR_OBSOLETE_SCRIPTS : []
+ scripts = UserVars.autostart_scripts.to_a + SetupFiles.new.get_settings.autostarts.to_a
+ scripts.reject { |name| obsolete.include?(name) }.uniq
end🤖 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 `@dependency.lic` around lines 943 - 952, The migration only clears
Settings['autostart'] but legacy entries in UserVars.autostart_scripts and
YAML-loaded autostarts still get launched; update the migration block to also
remove or filter legacy entries from UserVars.autostart_scripts and any
autostarts collection loaded from YAML (the same place that later
iterates/starts autostarts) so obsolete scripts are not relaunched before the
warning runs. Locate the code that reads/merges UserVars.autostart_scripts and
autostarts and either purge entries that match the obsolete autostart format or
set UserVars.autostart_scripts and the merged autostarts list to an
empty/filtered value in the migration so the subsequent startup loop will skip
them.
Comment on lines
+954
to
+970
| # --- Deprecated autostart helpers --- | ||
| # These functions were removed in 3.1.0. Thin stubs remain so that | ||
| # scripts or console calls get a clear message instead of NoMethodError. | ||
| def autostart(_script_names, _global = true) | ||
| echo("DEPRECATED: autostart() has been removed.") | ||
| echo("Use the 'autostarts' list in your YAML profile, or ;autostart add <script>") | ||
| end | ||
|
|
||
| def stop_autostart(script_names) | ||
| script_names = [script_names] unless script_names.is_a?(Array) | ||
| UserVars.autostart_scripts ||= [] | ||
| script_names.each { |script| UserVars.autostart_scripts.delete(script) } | ||
| end | ||
| def stop_autostart(_script_names) | ||
| echo("DEPRECATED: stop_autostart() has been removed.") | ||
| echo("Remove the script from 'autostarts' in your YAML profile, or ;autostart remove <script>") | ||
| end | ||
|
|
||
| def dependency_status | ||
| echo("Dependency version: #{$DEPENDENCY_VERSION}") | ||
| echo("Settings['autostart'] = #{Settings['autostart'].inspect}") | ||
| echo("UserVars.autostart_scripts = #{UserVars.autostart_scripts.inspect}") | ||
| echo("Resolved autostarts = #{$manager.autostarts.inspect}") if defined?($manager) | ||
| nil | ||
| end | ||
| end # Autostart helpers gate | ||
| def dependency_status | ||
| echo("DEPRECATED: dependency_status() has been removed.") | ||
| echo("Use ;autostart list to see active autostarts.") | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find in-repo calls that would exercise zero-arg or nonstandard arity on the deprecated helpers.
rg -nP --iglob '*.lic' '^(?!\s*#)\s*(?!def\s)(autostart|stop_autostart|dependency_status)\s*(\(\s*\))?\s*(#.*)?$'
rg -nP --iglob '*.lic' '(?<!def\s)\b(?:autostart|stop_autostart|dependency_status)\s*\([^)]*,'Repository: elanthia-online/dr-scripts
Length of output: 52
🏁 Script executed:
cat -n dependency.lic | sed -n '950,975p'Repository: elanthia-online/dr-scripts
Length of output: 1190
🏁 Script executed:
rg -n 'autostart|stop_autostart|dependency_status' dependency.licRepository: elanthia-online/dr-scripts
Length of output: 1294
🏁 Script executed:
# Broader search - look for any calls to these functions
rg -nB2 -A2 '\bautostart\s*\(' dependency.lic
rg -nB2 -A2 '\bstop_autostart\s*\(' dependency.lic
rg -nB2 -A2 '\bdependency_status\s*\(' dependency.licRepository: elanthia-online/dr-scripts
Length of output: 805
🏁 Script executed:
# Search across entire repo for calls to deprecated functions
rg -n '\bautostart\s*\(' --type-list
rg -n '\bautostart\s*\(' . --max-count=20
rg -n '\bstop_autostart\s*\(' . --max-count=20
rg -n '\bdependency_status\s*\(' . --max-count=20Repository: elanthia-online/dr-scripts
Length of output: 1208
🏁 Script executed:
# Check for any test files or scripts that might call these functions
fd -e lic -e rb -e py | head -20Repository: elanthia-online/dr-scripts
Length of output: 341
Make the deprecation stubs arity-tolerant.
These helpers are meant to turn legacy calls into a warning, but autostart and stop_autostart still raise ArgumentError for unsupported call shapes. A splat keeps any old invocation on the deprecation path instead.
Suggested fix
-def autostart(_script_names, _global = true)
+def autostart(*_args)
echo("DEPRECATED: autostart() has been removed.")
echo("Use the 'autostarts' list in your YAML profile, or ;autostart add <script>")
end
-def stop_autostart(_script_names)
+def stop_autostart(*_args)
echo("DEPRECATED: stop_autostart() has been removed.")
echo("Remove the script from 'autostarts' in your YAML profile, or ;autostart remove <script>")
end
-def dependency_status
+def dependency_status(*_args)
echo("DEPRECATED: dependency_status() has been removed.")
echo("Use ;autostart list to see active autostarts.")
end📝 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.
Suggested change
| # --- Deprecated autostart helpers --- | |
| # These functions were removed in 3.1.0. Thin stubs remain so that | |
| # scripts or console calls get a clear message instead of NoMethodError. | |
| def autostart(_script_names, _global = true) | |
| echo("DEPRECATED: autostart() has been removed.") | |
| echo("Use the 'autostarts' list in your YAML profile, or ;autostart add <script>") | |
| end | |
| def stop_autostart(script_names) | |
| script_names = [script_names] unless script_names.is_a?(Array) | |
| UserVars.autostart_scripts ||= [] | |
| script_names.each { |script| UserVars.autostart_scripts.delete(script) } | |
| end | |
| def stop_autostart(_script_names) | |
| echo("DEPRECATED: stop_autostart() has been removed.") | |
| echo("Remove the script from 'autostarts' in your YAML profile, or ;autostart remove <script>") | |
| end | |
| def dependency_status | |
| echo("Dependency version: #{$DEPENDENCY_VERSION}") | |
| echo("Settings['autostart'] = #{Settings['autostart'].inspect}") | |
| echo("UserVars.autostart_scripts = #{UserVars.autostart_scripts.inspect}") | |
| echo("Resolved autostarts = #{$manager.autostarts.inspect}") if defined?($manager) | |
| nil | |
| end | |
| end # Autostart helpers gate | |
| def dependency_status | |
| echo("DEPRECATED: dependency_status() has been removed.") | |
| echo("Use ;autostart list to see active autostarts.") | |
| end | |
| # --- Deprecated autostart helpers --- | |
| # These functions were removed in 3.1.0. Thin stubs remain so that | |
| # scripts or console calls get a clear message instead of NoMethodError. | |
| def autostart(*_args) | |
| echo("DEPRECATED: autostart() has been removed.") | |
| echo("Use the 'autostarts' list in your YAML profile, or ;autostart add <script>") | |
| end | |
| def stop_autostart(*_args) | |
| echo("DEPRECATED: stop_autostart() has been removed.") | |
| echo("Remove the script from 'autostarts' in your YAML profile, or ;autostart remove <script>") | |
| end | |
| def dependency_status(*_args) | |
| echo("DEPRECATED: dependency_status() has been removed.") | |
| echo("Use ;autostart list to see active autostarts.") | |
| end |
🤖 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 `@dependency.lic` around lines 954 - 970, Change the deprecation stubs to
accept arbitrary arguments so legacy call shapes don't raise ArgumentError:
update the method signatures for autostart and stop_autostart (and optionally
dependency_status) from def autostart(_script_names, _global = true) / def
stop_autostart(_script_names) to accept a splat like def autostart(*_args) and
def stop_autostart(*_args) (and def dependency_status(*_args) if you want it
arity-tolerant too); keep the existing echo messages and ignore the captured
args.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
redirectis unnecessary and the unwounded-body-part availability check was incorrectly blocking transfers. Non-Unity transfers (basic empathic link) still redirect as before.wait_for_passive_healingno longer fires when Regenerate is active, since Regenerate will heal all wounds given enough time. Without Regenerate, the timeout andhealmefallback still apply.unity_linkedflag is reset so patients get re-linked on recovery.Test plan
unity_linkedflagunity_linked: falsein initial patient state🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Changes
Deprecated
Tests