Skip to content

fix: skip plugin reload for deactivated plugins to preserve their tools#8740

Open
tangtaizong666 wants to merge 1 commit into
AstrBotDevs:masterfrom
tangtaizong666:fix/8582
Open

fix: skip plugin reload for deactivated plugins to preserve their tools#8740
tangtaizong666 wants to merge 1 commit into
AstrBotDevs:masterfrom
tangtaizong666:fix/8582

Conversation

@tangtaizong666

@tangtaizong666 tangtaizong666 commented Jun 12, 2026

Copy link
Copy Markdown

Fixes #8582

Saving the config of a deactivated plugin in the WebUI calls plugin_manager.reload(plugin_name) unconditionally (dashboard/routes/config.py). For a deactivated plugin, _terminate_plugin short-circuits, but _unbind_plugin still removes the plugin from star_map/star_registry and deletes all of its tools from llm_tools.func_list, while the subsequent load() skips instantiation (the plugin path is in inactivated_plugins). Tools registered via context.add_llm_tools() in the plugin's __init__ are therefore never re-added: they disappear from func_list until the plugin is re-enabled, and stale inactivated_llm_tools blacklist entries are left behind.

Modifications / 改动点

  • astrbot/core/star/star_manager.py: reload() now returns early when the specified plugin is listed in the persisted inactivated_plugins preference. A deactivated plugin has no instance to re-apply config to; the new config is picked up by the full reload that turn_on_plugin triggers when the plugin is re-enabled.

    • The guard intentionally checks the inactivated_plugins preference instead of metadata.activated: turn_on_plugin removes the entry from the preference before calling reload(), while the stale metadata still has activated=False at that moment, so checking the flag would break plugin re-enabling.
    • Fixing inside reload() covers every caller (config save, WebUI reload button, update_plugin), not just the config-save route.
  • tests/test_plugin_manager.py: regression tests for both paths — a deactivated plugin is left untouched (no terminate/unbind/load, stays in star_map/star_registry), and an activated plugin still goes through the full terminate → unbind → load cycle.

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

$ uv run pytest tests/test_plugin_manager.py -q
.......................................                                  [100%]
39 passed in 2.43s

(includes the two new regression tests; test_reload_skips_deactivated_plugin fails on master and passes with this change)

$ make pr-test-neo
==> Running Ruff format check
435 files already formatted
==> Running Ruff lint check
All checks passed!
==> Running pytest
............                                                             [100%]
12 passed in 3.83s
==> Starting smoke test on http://localhost:6185
==> Smoke test passed
==> PR checks completed successfully

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了"验证步骤"和"运行截图"

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Guard plugin reload against deactivated plugins so their tools remain registered and configuration is applied only when re-enabled.

Bug Fixes:

  • Prevent reloading deactivated plugins from unbinding them and removing their registered LLM tools while skipping re-instantiation.

Tests:

  • Add regression tests ensuring reload is skipped for deactivated plugins and still performs the full terminate–unbind–load cycle for activated plugins.

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend feature:plugin The bug / feature is about AstrBot plugin system. labels Jun 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a check in star_manager.py to skip reloading deactivated plugins, preventing their registered tools from being unbound and lost. It also adds comprehensive unit tests to verify that deactivated plugins skip the reload cycle while activated plugins undergo the full cycle. The review feedback suggests adding or [] when retrieving inactivated_plugins to handle cases where the key exists but is explicitly set to None, avoiding a potential TypeError during membership testing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

break

if specified_module_path:
inactivated_plugins = await sp.global_get("inactivated_plugins", [])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure defensive programming and robust handling of potential null/None values in the configuration, it is safer to append or [] when retrieving inactivated_plugins. If the key exists in the shared preferences but is explicitly set to null (or None), sp.global_get will return None, which would subsequently cause a TypeError when executing specified_module_path in inactivated_plugins.

Suggested change
inactivated_plugins = await sp.global_get("inactivated_plugins", [])
inactivated_plugins = await sp.global_get("inactivated_plugins", []) or []

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In the new reload guard, the log message uses specified_plugin_name, which can be None when reloading by module path or in bulk; consider deriving a name from smd or falling back to specified_module_path so the log line is always meaningful.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the new `reload` guard, the log message uses `specified_plugin_name`, which can be `None` when reloading by module path or in bulk; consider deriving a name from `smd` or falling back to `specified_module_path` so the log line is always meaningful.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@tangtaizong666

Copy link
Copy Markdown
Author

Responding to the two bot review suggestions after checking them against the codebase:

or [] guard on global_get("inactivated_plugins", []) — not applied. The preference is only ever written by turn_off_plugin / turn_on_plugin, both of which always store a list. The three pre-existing readers in the same file (load(), turn_off_plugin, turn_on_plugin) all use the identical bare await sp.global_get("inactivated_plugins", []) pattern with no or []; if a None value were actually possible it would already crash load() at startup before reload() could run. The new call site follows the established pattern.

specified_plugin_name possibly None in the log line — not possible. The guard sits inside if specified_module_path:, and specified_module_path is only assigned inside if specified_plugin_name:. reload() has no reload-by-module-path mode, and bulk reload (specified_plugin_name=None) leaves specified_module_path as None, which skips the guard entirely. At that log line the name is always set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend feature:plugin The bug / feature is about AstrBot plugin system. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 保存停用插件配置时无条件 reload(),导致 func_list 中全部工具被 _unbind_plugin 移除

1 participant