-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: session-level plugin disable rules not applied to event hooks and tool list #7975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f120ebb
0390c0c
a212f56
2b4f3b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -383,6 +383,7 @@ def _build_local_mode_prompt() -> str: | |
| def _filter_skills_for_current_config( | ||
| skills: list[SkillInfo], | ||
| cfg: dict, | ||
| session_disabled: set[str] | None = None, | ||
| ) -> list[SkillInfo]: | ||
| plugin_set = cfg.get("plugin_set", ["*"]) | ||
| allowed_plugins = ( | ||
|
|
@@ -404,7 +405,12 @@ def _filter_skills_for_current_config( | |
| plugin = plugin_by_root_dir.get(skill.plugin_name) | ||
| if not plugin or not plugin.activated: | ||
| continue | ||
| if plugin.reserved or allowed_plugins is None: | ||
| if plugin.reserved: | ||
| filtered.append(skill) | ||
| continue | ||
| if session_disabled and plugin.name in session_disabled: | ||
| continue | ||
| if allowed_plugins is None: | ||
| filtered.append(skill) | ||
| continue | ||
| if plugin.name is not None and plugin.name in allowed_plugins: | ||
|
|
@@ -422,6 +428,19 @@ async def _ensure_persona_and_skills( | |
| if not req.conversation: | ||
| return | ||
|
|
||
| from astrbot.core import sp | ||
|
|
||
| session_id = event.unified_msg_origin | ||
| session_plugin_config = await sp.get_async( | ||
| scope="umo", | ||
| scope_id=session_id, | ||
| key="session_plugin_config", | ||
| default={}, | ||
| ) | ||
| session_disabled = set( | ||
| session_plugin_config.get(session_id, {}).get("disabled_plugins", []) | ||
| ) | ||
|
|
||
| ( | ||
| persona_id, | ||
| persona, | ||
|
|
@@ -454,7 +473,7 @@ async def _ensure_persona_and_skills( | |
| runtime = cfg.get("computer_use_runtime", "local") | ||
| skill_manager = SkillManager() | ||
| skills = skill_manager.list_skills(active_only=True, runtime=runtime) | ||
| skills = _filter_skills_for_current_config(skills, cfg) | ||
| skills = _filter_skills_for_current_config(skills, cfg, session_disabled) | ||
|
|
||
| if skills: | ||
| if persona and persona.get("skills") is not None: | ||
|
|
@@ -898,33 +917,58 @@ async def _decorate_llm_request( | |
| _apply_workspace_extra_prompt(event, req) | ||
|
|
||
|
|
||
| def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None: | ||
| async def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None: | ||
| """根据事件中的插件设置,过滤请求中的工具列表。 | ||
|
|
||
| 注意:没有 handler_module_path 的工具(如 MCP 工具)会被保留, | ||
| 因为它们不属于任何插件,不应被插件过滤逻辑影响。 | ||
| """ | ||
| if event.plugins_name is not None and req.func_tool: | ||
| new_tool_set = ToolSet() | ||
| for tool in req.func_tool.tools: | ||
| if isinstance(tool, MCPTool): | ||
| # 保留 MCP 工具 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| mp = tool.handler_module_path | ||
| if not mp: | ||
| # 没有 plugin 归属信息的工具(如 subagent transfer_to_*) | ||
| # 不应受到会话插件过滤影响。 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| plugin = star_map.get(mp) | ||
| if not plugin: | ||
| # 无法解析插件归属时,保守保留工具,避免误过滤。 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| if plugin.name in event.plugins_name or plugin.reserved: | ||
| new_tool_set.add_tool(tool) | ||
| req.func_tool = new_tool_set | ||
| if not req.func_tool: | ||
| return | ||
|
|
||
| from astrbot.core import sp | ||
|
|
||
| session_id = event.unified_msg_origin | ||
| session_plugin_config = await sp.get_async( | ||
| scope="umo", | ||
| scope_id=session_id, | ||
| key="session_plugin_config", | ||
| default={}, | ||
| ) | ||
| session_disabled = set( | ||
| session_plugin_config.get(session_id, {}).get("disabled_plugins", []) | ||
| ) | ||
|
Comment on lines
+931
to
+940
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block of code for fetching session-level disabled plugins is identical to the one in _ensure_persona_and_skills (lines 433-442). Since both functions are called sequentially within build_main_agent, this results in redundant database queries. Consider fetching this configuration once in build_main_agent and passing it as an argument, or caching it within the event object to optimize performance. References
|
||
|
|
||
| global_whitelist = event.plugins_name # None 表示全部允许 | ||
|
|
||
| new_tool_set = ToolSet() | ||
| for tool in req.func_tool.tools: | ||
| if isinstance(tool, MCPTool): | ||
| # 保留 MCP 工具 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| mp = tool.handler_module_path | ||
| if not mp: | ||
| # 没有 plugin 归属信息的工具(如 subagent transfer_to_*) | ||
| # 不应受到会话插件过滤影响。 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| plugin = star_map.get(mp) | ||
| if not plugin: | ||
| # 无法解析插件归属时,保守保留工具,避免误过滤。 | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| if plugin.reserved: | ||
| new_tool_set.add_tool(tool) | ||
| continue | ||
| # 全局白名单过滤 | ||
| if global_whitelist is not None and plugin.name not in global_whitelist: | ||
| continue | ||
| # 会话级禁用过滤 | ||
| if plugin.name in session_disabled: | ||
| continue | ||
| new_tool_set.add_tool(tool) | ||
| req.func_tool = new_tool_set | ||
|
|
||
|
|
||
| async def _handle_webchat( | ||
|
|
@@ -1372,7 +1416,7 @@ async def build_main_agent( | |
| if not req.session_id: | ||
| req.session_id = event.unified_msg_origin | ||
|
|
||
| _plugin_tool_fix(event, req) | ||
| await _plugin_tool_fix(event, req) | ||
| await _apply_web_search_tools(event, req, plugin_context) | ||
|
|
||
| if config.llm_safety_mode: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider extracting shared helpers for session-disabled plugins and plugin enablement logic to remove duplication and keep the filtering rules in one place.
You can centralize the “session plugin disabling” concern and simplify the branching without changing behavior.
1. Extract session-level disabled plugins helper
Right now this logic is duplicated in
_ensure_persona_and_skillsand_plugin_tool_fix(and apparently incontext_utils.call_event_hook).Create a small helper and reuse it:
Then in
_ensure_persona_and_skills:And in
_plugin_tool_fix:You can mirror this in
context_utils.call_event_hookas well.2. Centralize plugin enablement rules
The rules (reserved → always allow, global whitelist, session disabled) are currently embedded separately in
_filter_skills_for_current_configand_plugin_tool_fix. A tiny predicate keeps this linear and shared:Use it in
_filter_skills_for_current_config:And in
_plugin_tool_fix:This keeps all current behavior (reserved always allowed, unknown plugin/tool preserved, MCP tools preserved, session and global filters respected) but removes the duplicated session config access and scattered condition sets, which should make future changes to the enablement rules safer and easier to reason about.