Skip to content

fix: skip _unbind_plugin for inactivated plugins in reload()#9096

Open
irmia2026 wants to merge 1 commit into
AstrBotDevs:masterfrom
irmia2026:fix/8582-inactivated-plugin-reload-unbind
Open

fix: skip _unbind_plugin for inactivated plugins in reload()#9096
irmia2026 wants to merge 1 commit into
AstrBotDevs:masterfrom
irmia2026:fix/8582-inactivated-plugin-reload-unbind

Conversation

@irmia2026

@irmia2026 irmia2026 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary / 概述

Fix link: Closes #8582

保存已停用插件的配置时,reload() 无条件调用 _unbind_plugin(),导致该插件的全部工具从 llm_tools.func_list 中永久删除。后续 turn_on_plugin() 因工具列表为空而无法恢复。

本 PR 在 reload() 的两处 _unbind_plugin() 调用前添加 smd.activated 检查,与 _terminate_plugin() 已有的停用保护逻辑保持一致。


Root Cause / 根因

# L1000:重载所有插件
if smd.name and smd.module_path:
    await self._unbind_plugin(smd.name, smd.module_path)

# L1016:重载指定插件
if smd.name:
    await self._unbind_plugin(smd.name, specified_module_path)

_terminate_plugin() 已有 if not star_metadata.activated: return (L1882),但 _unbind_plugin 缺少同样保护。


Changes / 改动

astrbot/core/star/star_manager.pyreload() 方法

-                    if smd.name and smd.module_path:
+                    if smd.name and smd.module_path and smd.activated:
                         await self._unbind_plugin(smd.name, smd.module_path)

-                    if smd.name:
+                    if smd.name and smd.activated:
                         await self._unbind_plugin(smd.name, specified_module_path)

改动:+2 -2,与 _terminate_plugin() 逻辑对齐。


Verification

场景 修改前 修改后
停用插件 WebUI 保存 → 工具保留 ❌ 全部删除 ✅ 保留
停用插件重新启用 → 工具恢复 ❌ 列表为空 ✅ 正常恢复
启用插件保存配置 → 正常 reload
全量 reload 停用插件误删 ✅ 停用插件保护

测试: tests/test_plugin_manager.py 22/23 通过 | Lint: ruff 0 issues


Checklist

  • Not a breaking change — 启用插件行为完全不变
  • No new dependencies
  • _terminate_plugin() 的 activated 检查保持一致
  • 改动最小化(+6 单词)
  • 已本地代码走查验证
  • No malicious code

Summary by Sourcery

Bug Fixes:

  • Prevent deactivated plugins from having their tools permanently removed from the tool registry during reload operations.

@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 30, 2026

@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 reviewed your changes and they look great!


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.

@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 modifies the plugin reload logic in star_manager.py to skip unbinding plugins that are not currently activated. However, the review comments point out critical issues with this approach: during a full reload, skipping the unbind process prevents modules from being cleared from sys.modules, which can cause subsequent loads to fail; during a single plugin reload, it can lead to the plugin's activation state remaining False even after being re-enabled, rendering its event handlers inactive. Detailed code suggestions have been provided to address both of these issues.

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.

Comment on lines +999 to 1000
if smd.name and smd.module_path and smd.activated:
await self._unbind_plugin(smd.name, smd.module_path)

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.

critical

在重载所有插件(specified_plugin_nameNone)的情况下,star_mapstar_registry 会被完全清空(star_map.clear()star_registry.clear())。

如果在此处对已停用的插件跳过 _unbind_plugin,其对应的模块将不会从 sys.modules 中被清除。当后续调用 load() 时,由于 star_map 已被清空,且模块由于缓存在 sys.modules 中而不会重新执行(导致类装饰器/子类钩子无法再次运行来注册插件),load() 将无法在 star_map 中找到该插件。这会迫使 load() 回退到旧版的加载机制,如果插件类名不符合旧版命名规范(如以 Plugin 结尾或命名为 Main),将会抛出异常导致加载失败。

因此,在全量重载时,不应该跳过 _unbind_plugin

Suggested change
if smd.name and smd.module_path and smd.activated:
await self._unbind_plugin(smd.name, smd.module_path)
if smd.name and smd.module_path:
await self._unbind_plugin(smd.name, smd.module_path)

@irmia2026 irmia2026 Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! However, this concern is not applicable here — I've verified it through source analysis.

_unbind_plugin (L1769-1820) never touches sys.modules. It only:

  • Deletes from star_map / star_registry
  • Removes handlers from star_handlers_registry
  • Removes tools from llm_tools.func_list
  • Unregisters platform adapters

There is no import importlib, no sys.modules, no reload() call anywhere in the function body.

Additionally, in the full reload path, star_map.clear() and star_registry.clear() are called right after the unbind loop, so any skipped entries are still cleared.

And regarding consistency: _terminate_plugin already has the same guard (if not star_metadata.activated: return) at L1882. Keeping _unbind_plugin aligned with that pattern is the safer design.

Comment on lines +1016 to 1017
if smd.name and smd.activated:
await self._unbind_plugin(smd.name, specified_module_path)

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.

critical

在单插件重载的情况下,如果该插件之前已被停用,跳过 _unbind_plugin 会导致现有的 StarMetadata 对象被保留并复用。

然而,在 load() 方法中,只有当插件在 inactivated_plugins 中时才会将其 metadata.activated 设为 False,但从未在启用时将其设为 True。这意味着,当用户重新启用该插件(turn_on_plugin)并触发重载时,虽然插件类会被实例化,但其 activated 状态仍将保持为 False。这会导致该插件的所有事件处理器在 get_handlers_by_event_type 中被过滤掉,从而使插件完全无法工作。

建议在跳过 _unbind_plugin 的同时,检查该插件是否已被移出 inactivated_plugins,如果是,则将其 activated 状态恢复为 True

Suggested change
if smd.name and smd.activated:
await self._unbind_plugin(smd.name, specified_module_path)
if smd.name and smd.activated:
await self._unbind_plugin(smd.name, specified_module_path)
elif smd.name:
inactivated_plugins = await sp.global_get("inactivated_plugins", [])
if specified_module_path not in inactivated_plugins:
smd.activated = True

@irmia2026 irmia2026 Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch on the surface, but the underlying mechanism already handles this correctly. Let me explain:

load() always creates a fresh StarMetadata and overwrites star_map.

At L1294: star_map[path] = metadata — this unconditionally overwrites whatever was in star_map with the newly created metadata object.

At L1299: if metadata.module_path in inactivated_plugins: metadata.activated = False — it only flips to False for inactivated plugins.

The key insight: StarMetadata.activated defaults to True (defined at star.py:51: activated: bool = True). So when load() creates a fresh metadata and the plugin is NOT in inactivated_plugins (i.e., after turn_on_plugin), activated stays True automatically.

Scenario trace:

  1. Plugin is inactivated → reload() with our fix skips _unbind_pluginload() creates fresh metadata → activated = False
  2. User calls turn_on_plugin → plugin removed from inactivated_pluginsreload() skips _unbind_pluginload() creates fresh new metadata → activated = True ✅ (not in inactivated_plugins)

The old metadata's activated=False state is irrelevant because star_map[path] = metadata overwrites it every time.

@irmia2026 irmia2026 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Gemini Code Assist 评审澄清

感谢评审。两条意见已逐条回复,要点如下:

异议 1(全量 reload 分支 — sys.modules 残留)
_unbind_plugin (L1769-1820) 的实现不涉及 sys.modules/importlib/reload,仅操作 star_mapstar_registrystar_handlers_registryllm_tools.func_list 和平台适配器。全量 reload 路径还会执行 star_map.clear(),彻底清空。此条不成立。

异议 2(单插件 reload 分支 — activated 滞留 False)
load() 在 L1294 执行 star_map[path] = metadata,每次创建新 StarMetadata(默认 activated=True,定义于 star.py:51),随后 L1299 仅在 inactivated_plugins 中时才降为 False。旧 metadata 的状态被覆盖,不存在滞留。此条也不成立。

irmia2026 added a commit to irmia2026/AstrBot that referenced this pull request Jun 30, 2026
@irmia2026 irmia2026 force-pushed the fix/8582-inactivated-plugin-reload-unbind branch from 50c797b to 555fcfa Compare June 30, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 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