fix: session-level plugin disable rules not applied to event hooks and tool list

[lingyun14beta]

Fixes #7949

When a plugin is disabled via session-level custom rules, its registered event hooks (OnLLMRequestEvent, OnLLMResponseEvent, OnDecoratingResultEvent, etc.) and function call tools are still executed/mounted, making the disable rule ineffective.

Root cause:

• call_event_hook in context_utils.py only checks the global event.plugins_name when triggering hooks, without awareness of session-level disabled plugins
• _plugin_tool_fix in astr_main_agent.py skips all filtering when plugins_name is None (i.e. all plugins globally enabled), ignoring session-level disabled plugins
• _filter_skills_for_current_config similarly only filters against the global whitelist, not session-level disabled plugins

Modifications / 改动点

• astrbot/core/pipeline/context_utils.py: added session-level disable check in call_event_hook
• astrbot/core/astr_main_agent.py: changed _plugin_tool_fix to async and added session-level filtering; added session_disabled parameter to _filter_skills_for_current_config; pass session disabled list in _ensure_persona_and_skills

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

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

---

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.txt 和 pyproject.toml 文件相应位置。

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

Summary by Sourcery

Ensure session-level plugin disable rules are respected when invoking plugins during conversation handling.

Bug Fixes:

• Prevent event hooks from firing for plugins that are disabled at the session level.
• Exclude tools belonging to session-disabled plugins from the function/tool list while preserving reserved and unowned tools.
• Filter skills based on both global plugin configuration and per-session disabled plugins to avoid invoking disabled plugin capabilities.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The session_plugin_config structure is being treated as a dict-of-dicts (session_plugin_config.get(session_id, {})...) in _ensure_persona_and_skills, _plugin_tool_fix, and call_event_hook, even though scope_id=session_id is already passed to sp.get_async; double-check whether this should instead read directly from session_plugin_config.get("disabled_plugins", []) to avoid an extra nesting assumption.
• The logic to load session_plugin_config and compute session_disabled is duplicated in at least three places; consider extracting a small helper (e.g. get_session_disabled_plugins(session_id)) to keep the behavior consistent and make future changes less error-prone.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `session_plugin_config` structure is being treated as a dict-of-dicts (`session_plugin_config.get(session_id, {})...`) in `_ensure_persona_and_skills`, `_plugin_tool_fix`, and `call_event_hook`, even though `scope_id=session_id` is already passed to `sp.get_async`; double-check whether this should instead read directly from `session_plugin_config.get("disabled_plugins", [])` to avoid an extra nesting assumption.
- The logic to load `session_plugin_config` and compute `session_disabled` is duplicated in at least three places; consider extracting a small helper (e.g. `get_session_disabled_plugins(session_id)`) to keep the behavior consistent and make future changes less error-prone.

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/context_utils.py" line_range="97-98" />
<code_context>
+        key="session_plugin_config",
+        default={},
+    )
+    session_disabled = session_plugin_config.get(session_id, {}).get(
+        "disabled_plugins", []
+    )
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider normalizing `session_disabled` to a set for efficient membership checks.

In `call_event_hook`, `session_disabled` is still a list and used with `plugin.name in session_disabled` inside the loop. Converting it to a set once (as in `_plugin_tool_fix`) would give O(1) membership checks and keep the filtering behavior consistent across both call sites.

Suggested implementation:

```python
    """
    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", [])
    )

    """
    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", [])
    )

```

1. In `call_event_hook`, update any code that assumes `session_disabled` is a list (e.g. list methods like `.append` or `.extend`) to work with a set, or convert locally with `set(session_disabled)` if mutation as a list is required.
2. Ensure `plugin.name in session_disabled` in `call_event_hook` and `_plugin_tool_fix` now rely on the set-typed `session_disabled` (i.e. remove any redundant `set(...)` wrapping at those call sites to avoid unnecessary conversions).
3. If `session_plugin_config` is not actually keyed by `session_id` (i.e. it is already the per-session dict), then replace `session_plugin_config.get(session_id, {})` with just `session_plugin_config` in both places above to avoid a double lookup.
</issue_to_address>

### Comment 2
<location path="astrbot/core/astr_main_agent.py" line_range="440" />
<code_context>
+        key="session_plugin_config",
+        default={},
+    )
+    session_disabled = set(
+        session_plugin_config.get(session_id, {}).get("disabled_plugins", [])
+    )
</code_context>
<issue_to_address>
**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_skills` and `_plugin_tool_fix` (and apparently in `context_utils.call_event_hook`).

Create a small helper and reuse it:

```python
# somewhere shared, e.g. astrbot/core/session_plugins.py
from astrbot.core import sp

async def get_session_disabled_plugins(session_id: str) -> set[str]:
    session_plugin_config = await sp.get_async(
        scope="umo",
        scope_id=session_id,
        key="session_plugin_config",
        default={},
    )
    return set(session_plugin_config.get(session_id, {}).get("disabled_plugins", []))
```

Then in `_ensure_persona_and_skills`:

```python
from astrbot.core.session_plugins import get_session_disabled_plugins

async def _ensure_persona_and_skills(...):
    ...
    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    ...
    skills = _filter_skills_for_current_config(skills, cfg, session_disabled)
```

And in `_plugin_tool_fix`:

```python
from astrbot.core.session_plugins import get_session_disabled_plugins

async def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
    if not req.func_tool:
        return

    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    global_whitelist = event.plugins_name
    ...
```

You can mirror this in `context_utils.call_event_hook` as well.

### 2. Centralize plugin enablement rules

The rules (reserved → always allow, global whitelist, session disabled) are currently embedded separately in `_filter_skills_for_current_config` and `_plugin_tool_fix`. A tiny predicate keeps this linear and shared:

```python
def is_plugin_enabled(
    plugin,
    *,
    global_whitelist: set[str] | None,
    session_disabled: set[str] | None,
) -> bool:
    if plugin.reserved:
        return True
    if session_disabled and plugin.name in session_disabled:
        return False
    if global_whitelist is None:
        return True
    return plugin.name in global_whitelist
```

Use it in `_filter_skills_for_current_config`:

```python
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 = (
        None
        if not isinstance(plugin_set, list) or "*" in plugin_set
        else {str(name) for name in plugin_set}
    )

    plugin_by_root_dir = {
        metadata.root_dir_name: metadata
        for metadata in star_registry
    }

    filtered: list[SkillInfo] = []
    for skill in skills:
        if skill.source_type != "plugin":
            filtered.append(skill)
            continue

        plugin = plugin_by_root_dir.get(skill.plugin_name)
        if not plugin or not plugin.activated:
            continue

        if is_plugin_enabled(
            plugin,
            global_whitelist=allowed_plugins,
            session_disabled=session_disabled,
        ):
            filtered.append(skill)

    return filtered
```

And in `_plugin_tool_fix`:

```python
async def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
    if not req.func_tool:
        return

    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    global_whitelist = set(event.plugins_name) if event.plugins_name is not None else None

    new_tool_set = ToolSet()
    for tool in req.func_tool.tools:
        if isinstance(tool, MCPTool):
            new_tool_set.add_tool(tool)
            continue

        mp = tool.handler_module_path
        if not mp:
            new_tool_set.add_tool(tool)
            continue

        plugin = star_map.get(mp)
        if not plugin:
            new_tool_set.add_tool(tool)
            continue

        if is_plugin_enabled(
            plugin,
            global_whitelist=global_whitelist,
            session_disabled=session_disabled,
        ):
            new_tool_set.add_tool(tool)

    req.func_tool = new_tool_set
```

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.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

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_skills and _plugin_tool_fix (and apparently in context_utils.call_event_hook).

Create a small helper and reuse it:

# somewhere shared, e.g. astrbot/core/session_plugins.py
from astrbot.core import sp

async def get_session_disabled_plugins(session_id: str) -> set[str]:
    session_plugin_config = await sp.get_async(
        scope="umo",
        scope_id=session_id,
        key="session_plugin_config",
        default={},
    )
    return set(session_plugin_config.get(session_id, {}).get("disabled_plugins", []))

Then in _ensure_persona_and_skills:

from astrbot.core.session_plugins import get_session_disabled_plugins

async def _ensure_persona_and_skills(...):
    ...
    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    ...
    skills = _filter_skills_for_current_config(skills, cfg, session_disabled)

And in _plugin_tool_fix:

from astrbot.core.session_plugins import get_session_disabled_plugins

async def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
    if not req.func_tool:
        return

    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    global_whitelist = event.plugins_name
    ...

You can mirror this in context_utils.call_event_hook as well.

2. Centralize plugin enablement rules

The rules (reserved → always allow, global whitelist, session disabled) are currently embedded separately in _filter_skills_for_current_config and _plugin_tool_fix. A tiny predicate keeps this linear and shared:

def is_plugin_enabled(
    plugin,
    *,
    global_whitelist: set[str] | None,
    session_disabled: set[str] | None,
) -> bool:
    if plugin.reserved:
        return True
    if session_disabled and plugin.name in session_disabled:
        return False
    if global_whitelist is None:
        return True
    return plugin.name in global_whitelist

Use it in _filter_skills_for_current_config:

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 = (
        None
        if not isinstance(plugin_set, list) or "*" in plugin_set
        else {str(name) for name in plugin_set}
    )

    plugin_by_root_dir = {
        metadata.root_dir_name: metadata
        for metadata in star_registry
    }

    filtered: list[SkillInfo] = []
    for skill in skills:
        if skill.source_type != "plugin":
            filtered.append(skill)
            continue

        plugin = plugin_by_root_dir.get(skill.plugin_name)
        if not plugin or not plugin.activated:
            continue

        if is_plugin_enabled(
            plugin,
            global_whitelist=allowed_plugins,
            session_disabled=session_disabled,
        ):
            filtered.append(skill)

    return filtered

And in _plugin_tool_fix:

async def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
    if not req.func_tool:
        return

    session_id = event.unified_msg_origin
    session_disabled = await get_session_disabled_plugins(session_id)
    global_whitelist = set(event.plugins_name) if event.plugins_name is not None else None

    new_tool_set = ToolSet()
    for tool in req.func_tool.tools:
        if isinstance(tool, MCPTool):
            new_tool_set.add_tool(tool)
            continue

        mp = tool.handler_module_path
        if not mp:
            new_tool_set.add_tool(tool)
            continue

        plugin = star_map.get(mp)
        if not plugin:
            new_tool_set.add_tool(tool)
            continue

        if is_plugin_enabled(
            plugin,
            global_whitelist=global_whitelist,
            session_disabled=session_disabled,
        ):
            new_tool_set.add_tool(tool)

    req.func_tool = new_tool_set

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.

[gemini-code-assist]

Code Review

This pull request implements session-level plugin disabling by filtering skills, tools, and event hooks based on a session-specific configuration retrieved from storage. Feedback suggests refactoring the handler filtering logic in context_utils.py to utilize the existing SessionPluginManager abstraction and optimizing astr_main_agent.py to eliminate redundant database queries for the session configuration during the agent build process.

[gemini-code-assist]

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

1. When implementing similar functionality for different cases, refactor the logic into a shared helper function to avoid code duplication.

[lingyun14beta]

这两个要修改动都好大：（
可能可以单开一个pr(?)