fix:fix the issue where incomplete cleanup of residual plugins occurs…#5462
Merged
Soulter merged 3 commits intoAstrBotDevs:masterfrom Feb 26, 2026
Merged
fix:fix the issue where incomplete cleanup of residual plugins occurs…#5462Soulter merged 3 commits intoAstrBotDevs:masterfrom
Soulter merged 3 commits intoAstrBotDevs:masterfrom
Conversation
… in the failed loading of plugins
dosubot
bot
added
the
size:M
Contributor
Summary of ChangesHello @Waterwzy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the reliability of plugin reloading by addressing an issue where incomplete cleanup of failed plugins led to persistent errors. By introducing a robust mechanism to clear associated modules, handlers, and tools, the system can now perform a clean slate reload, enhancing overall stability and user experience when managing plugins. This change ensures that previously failed plugins can be successfully reloaded without interference from their prior, incomplete state. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 新的清理逻辑周围多出来的
#'''注释标记看起来像是调试时遗留下来的脚手架代码，应当移除以避免造成困惑。 plugin_root_name = "data.plugins."这个字符串目前是硬编码的；如果项目里已经有插件包根路径的集中配置或常量，建议在这里复用，以避免未来产生偏差。
给 AI Agent 的提示
请根据本次代码评审中的评论进行修改：
## 总体意见
- 新的清理逻辑周围多出来的 `#'''` 注释标记看起来像是调试时遗留下来的脚手架代码，应当移除以避免造成困惑。
- `plugin_root_name = "data.plugins."` 这个字符串目前是硬编码的；如果项目里已经有插件包根路径的集中配置或常量，建议在这里复用，以避免未来产生偏差。
## 具体评论
### 评论 1
<location path="astrbot/core/star/star_manager.py" line_range="406" />
<code_context>
+ if not dir_name in self.failed_plugin_dict:
+ return False, "插件不存在于失败列表中"
+ #'''
+ plugin_root_name = "data.plugins."
+ # 清理 sys.modules
+ for key in list(sys.modules.keys()):
</code_context>
<issue_to_address>
**issue (complexity):** 建议将插件清理代码块提取到一个单独的辅助方法中，这样可以让 `reload_failed_plugin` 更专注于流程编排，也更方便复用。
你可以通过将清理逻辑提取到一个独立的辅助方法中来降低新增的复杂度，同时让 `reload_failed_plugin` 只专注于流程编排。如果之后需要从 `reload` 中复用，同样会非常简单。
例如：
```python
def _cleanup_plugin_state(self, dir_name: str) -> None:
plugin_root_name = "data.plugins."
# 清理 sys.modules
for key in list(sys.modules.keys()):
if key.startswith(f"{plugin_root_name}{dir_name}"):
logger.info(f"清除了插件{dir_name}中的{key}模块")
del sys.modules[key]
possible_paths = [
f"{plugin_root_name}{dir_name}.main",
f"{plugin_root_name}{dir_name}.{dir_name}",
]
# 清理 handlers
for path in possible_paths:
handlers = star_handlers_registry.get_handlers_by_module_name(path)
for handler in handlers:
star_handlers_registry.remove(handler)
logger.info(f"清理处理器: {handler.handler_name}")
# 清理工具
for tool in list(llm_tools.func_list):
if tool.handler_module_path in possible_paths:
llm_tools.func_list.remove(tool)
logger.info(f"清理工具: {tool.name}")
```
然后 `reload_failed_plugin` 会变得更简单，也更单一职责：
```python
async def reload_failed_plugin(self, dir_name):
async with self._pm_lock:
if dir_name not in self.failed_plugin_dict:
return False, "插件不存在于失败列表中"
self._cleanup_plugin_state(dir_name)
success, error = await self.load(specified_dir_name=dir_name)
if success:
self.failed_plugin_dict.pop(dir_name, None)
if not self.failed_plugin_dict:
self.failed_plugin_info = ""
return success, None
return False, error
```
如果之后在普通的 `reload` 流程中也需要同样的清理逻辑，只要调用 `self._cleanup_plugin_state(dir_name)` 即可，而不需要再次内联这些逻辑。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈持续改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The stray
#'''comment markers around the new cleanup logic look like leftover debugging scaffolding and should be removed to avoid confusion. - The
plugin_root_name = "data.plugins."string is hardcoded; if there is a central configuration or constant for the plugin package root, consider reusing it here to avoid future drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The stray `#'''` comment markers around the new cleanup logic look like leftover debugging scaffolding and should be removed to avoid confusion.
- The `plugin_root_name = "data.plugins."` string is hardcoded; if there is a central configuration or constant for the plugin package root, consider reusing it here to avoid future drift.
## Individual Comments
### Comment 1
<location path="astrbot/core/star/star_manager.py" line_range="406" />
<code_context>
+ if not dir_name in self.failed_plugin_dict:
+ return False, "插件不存在于失败列表中"
+ #'''
+ plugin_root_name = "data.plugins."
+ # 清理 sys.modules
+ for key in list(sys.modules.keys()):
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the plugin cleanup block into a dedicated helper method to keep `reload_failed_plugin` focused on orchestration and easier to reuse.
You can reduce the added complexity by extracting the cleanup logic into a dedicated helper and keeping `reload_failed_plugin` focused on orchestration. That also makes reuse from `reload` trivial if needed later.
For example:
```python
def _cleanup_plugin_state(self, dir_name: str) -> None:
plugin_root_name = "data.plugins."
# 清理 sys.modules
for key in list(sys.modules.keys()):
if key.startswith(f"{plugin_root_name}{dir_name}"):
logger.info(f"清除了插件{dir_name}中的{key}模块")
del sys.modules[key]
possible_paths = [
f"{plugin_root_name}{dir_name}.main",
f"{plugin_root_name}{dir_name}.{dir_name}",
]
# 清理 handlers
for path in possible_paths:
handlers = star_handlers_registry.get_handlers_by_module_name(path)
for handler in handlers:
star_handlers_registry.remove(handler)
logger.info(f"清理处理器: {handler.handler_name}")
# 清理工具
for tool in list(llm_tools.func_list):
if tool.handler_module_path in possible_paths:
llm_tools.func_list.remove(tool)
logger.info(f"清理工具: {tool.name}")
```
Then `reload_failed_plugin` becomes simpler and single‑purpose:
```python
async def reload_failed_plugin(self, dir_name):
async with self._pm_lock:
if dir_name not in self.failed_plugin_dict:
return False, "插件不存在于失败列表中"
self._cleanup_plugin_state(dir_name)
success, error = await self.load(specified_dir_name=dir_name)
if success:
self.failed_plugin_dict.pop(dir_name, None)
if not self.failed_plugin_dict:
self.failed_plugin_info = ""
return success, None
return False, error
```
If you later need the same cleanup during a normal `reload`, you can just call `self._cleanup_plugin_state(dir_name)` there as well instead of re‑embedding the logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
dosubot
bot
added
the
area:core
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
astrbot/core/star/star_manager.py
Outdated
Comment on lines
406
to
429
| plugin_root_name = "data.plugins." | ||
| # 清理 sys.modules | ||
| for key in list(sys.modules.keys()): | ||
| if key.startswith(f"{plugin_root_name}{dir_name}"): | ||
| logger.info(f"清除了插件{dir_name}中的{key}模块") | ||
| del sys.modules[key] | ||
|
|
||
| possible_paths = [ | ||
| f"{plugin_root_name}{dir_name}.main", | ||
| f"{plugin_root_name}{dir_name}.{dir_name}", | ||
| ] | ||
|
|
||
| # 清理 handlers | ||
| for path in possible_paths: | ||
| handlers = star_handlers_registry.get_handlers_by_module_name(path) | ||
| for handler in handlers: | ||
| star_handlers_registry.remove(handler) | ||
| logger.info(f"清理处理器: {handler.handler_name}") | ||
|
|
||
| # 清理工具 | ||
| for tool in list(llm_tools.func_list): | ||
| if tool.handler_module_path in possible_paths: | ||
| llm_tools.func_list.remove(tool) | ||
| logger.info(f"清理工具: {tool.name}") |
Contributor
There was a problem hiding this comment.
plugin_root_name 被硬编码为 data.plugins.，这在重载失败的内置插件（reserved plugin，来自 astrbot/builtin_stars）时会引发问题。代码应该判断插件是否为内置插件，并使用正确的模块路径前缀。这也适用于为清理 handlers 和 tools 构建 possible_paths。
is_reserved = os.path.exists(os.path.join(self.reserved_plugin_path, dir_name))
plugin_root_name = "astrbot.builtin_stars." if is_reserved else "data.plugins."
# 清理 sys.modules
for key in list(sys.modules.keys()):
if key.startswith(f"{plugin_root_name}{dir_name}"):
logger.info(f"清除了插件{dir_name}中的{key}模块")
del sys.modules[key]
possible_paths = [
f"{plugin_root_name}{dir_name}.main",
f"{plugin_root_name}{dir_name}.{dir_name}",
]
# 清理 handlers
for path in possible_paths:
handlers = star_handlers_registry.get_handlers_by_module_name(path)
for handler in handlers:
star_handlers_registry.remove(handler)
logger.info(f"清理处理器: {handler.handler_name}")
# 清理工具
for tool in list(llm_tools.func_list):
if tool.handler_module_path in possible_paths:
llm_tools.func_list.remove(tool)
logger.info(f"清理工具: {tool.name}")
astrbot/core/star/star_manager.py
Outdated
| return False, "插件不存在于失败列表中" | ||
| if not dir_name in self.failed_plugin_dict: | ||
| return False, "插件不存在于失败列表中" | ||
| #''' |
Contributor
There was a problem hiding this comment.
此处的 #''' 标记以及在 430 行的类似标记，看起来是调试时遗留的产物，建议移除以保持代码整洁。
astrbot/core/star/star_manager.py
Outdated
| if tool.handler_module_path in possible_paths: | ||
| llm_tools.func_list.remove(tool) | ||
| logger.info(f"清理工具: {tool.name}") | ||
| #''' |
Contributor
There was a problem hiding this comment.
此处的 #''' 标记看起来是调试时遗留的产物，建议移除以保持代码整洁。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出一些高层反馈：
- 建议复用或扩展现有的
_purge_modules逻辑，而不是新增一个与模块清理行为部分重复的_cleanup_plugin_state，这样可以让模块移除的语义保持集中且一致。 possible_paths列表目前只覆盖<dir_name>.main和<dir_name>.<dir_name>；如果插件可以从其他模块暴露 handlers/tools，你可能需要一种更通用的方式来推导所有相关的模块路径，以避免留下未清理的 handlers/tools。- 在
_cleanup_plugin_state中，每次重载失败都扫描整个sys.modules在更大的部署中可能会比较昂贵；如果这条路径会被频繁触发，可以考虑进一步收窄搜索范围（例如使用已加载插件模块的缓存列表）。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider reusing or extending the existing `_purge_modules` logic rather than introducing a separate `_cleanup_plugin_state` that partially duplicates module-cleanup behavior, so module-removal semantics stay centralized and consistent.
- The `possible_paths` list only covers `<dir_name>.main` and `<dir_name>.<dir_name>`; if plugins can expose handlers/tools from other modules, you may want a more general way to derive all relevant module paths to avoid leaving residual handlers/tools.
- In `_cleanup_plugin_state`, scanning all of `sys.modules` on every failed reload may become expensive in larger deployments; consider restricting the search further (e.g., using a cached list of loaded plugin modules) if this path is expected to be hit frequently.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的审查。
Original comment in English
Hey - I've left some high level feedback:
- Consider reusing or extending the existing
_purge_moduleslogic rather than introducing a separate_cleanup_plugin_statethat partially duplicates module-cleanup behavior, so module-removal semantics stay centralized and consistent. - The
possible_pathslist only covers<dir_name>.mainand<dir_name>.<dir_name>; if plugins can expose handlers/tools from other modules, you may want a more general way to derive all relevant module paths to avoid leaving residual handlers/tools. - In
_cleanup_plugin_state, scanning all ofsys.moduleson every failed reload may become expensive in larger deployments; consider restricting the search further (e.g., using a cached list of loaded plugin modules) if this path is expected to be hit frequently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider reusing or extending the existing `_purge_modules` logic rather than introducing a separate `_cleanup_plugin_state` that partially duplicates module-cleanup behavior, so module-removal semantics stay centralized and consistent.
- The `possible_paths` list only covers `<dir_name>.main` and `<dir_name>.<dir_name>`; if plugins can expose handlers/tools from other modules, you may want a more general way to derive all relevant module paths to avoid leaving residual handlers/tools.
- In `_cleanup_plugin_state`, scanning all of `sys.modules` on every failed reload may become expensive in larger deployments; consider restricting the search further (e.g., using a cached list of loaded plugin modules) if this path is expected to be hit frequently.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Soulter
approved these changes
Feb 26, 2026
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
2 participants
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.
… in the failed loading of plugins
Modifications / 改动点
Close:#5461
在#5334 #5043 中，失败插件的重载均没有移除插件的模块，handlers和插件工具
astrbot/core/star/star_manager.pyScreenshots or Test Results / 运行截图或测试结果
在插件初始化时，raise一个RuntimeError，显示：
修改raise的消息，重载插件，显示：
说明astrbot正确识别了插件的更改。
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
确保在重新加载之前对失败的插件进行完整清理，以便后续加载使用全新的状态。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Ensure failed plugins are fully cleaned up before reload so subsequent loads use a fresh state.
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
确保在重新加载之前对失败的插件进行完整清理，以便后续加载使用全新的状态。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Ensure failed plugins are fully cleaned up before reload so subsequent loads use a fresh state.
Bug Fixes:
Enhancements: