mirror of
https://github.com/AstrBotDevs/AstrBot
synced 2026-07-15 17:30:13 +08:00
ae44b912fc
* feat(func-tool): add per-tool permission management * feat(dashboard): add permission column to function tool table * refactor(dashboard): encapsulate tool actions into composable and unify permission UI * style: format code * fix: guard sync handler, simplify sp access, skip builtin instantiation
238 lines
8.5 KiB
Python
238 lines
8.5 KiB
Python
"""Tests for tool conflict resolution in ToolSet.add_tool and FunctionToolManager.
|
|
|
|
This module tests the fix for issue #5821: when an MCP external tool shares a name
|
|
with a disabled built-in tool, the MCP tool should not be removed as collateral damage.
|
|
"""
|
|
|
|
import pytest
|
|
|
|
from astrbot.core.agent.tool import FunctionTool, ToolSet
|
|
from astrbot.core.provider.func_tool_manager import FunctionToolManager
|
|
|
|
|
|
def make_tool(name: str, active: bool = True) -> FunctionTool:
|
|
"""Create a simple FunctionTool for testing."""
|
|
return FunctionTool(
|
|
name=name,
|
|
description=f"Test tool {name}",
|
|
parameters={"type": "object", "properties": {}},
|
|
active=active,
|
|
)
|
|
|
|
|
|
class TestToolSetAddTool:
|
|
"""Tests for ToolSet.add_tool conflict resolution."""
|
|
|
|
def test_new_tool_active_existing_inactive_overwrites(self):
|
|
"""Active tool should overwrite inactive tool with same name."""
|
|
toolset = ToolSet()
|
|
toolset.add_tool(make_tool("web_search", active=False))
|
|
toolset.add_tool(make_tool("web_search", active=True))
|
|
|
|
assert len(toolset.tools) == 1
|
|
assert toolset.tools[0].active is True
|
|
|
|
def test_new_tool_inactive_existing_active_preserves_existing(self):
|
|
"""Inactive tool should NOT overwrite active tool with same name."""
|
|
toolset = ToolSet()
|
|
toolset.add_tool(make_tool("web_search", active=True))
|
|
toolset.add_tool(make_tool("web_search", active=False))
|
|
|
|
assert len(toolset.tools) == 1
|
|
assert toolset.tools[0].active is True
|
|
|
|
def test_both_active_last_one_wins(self):
|
|
"""When both tools are active, the new one should overwrite."""
|
|
toolset = ToolSet()
|
|
first = make_tool("web_search", active=True)
|
|
second = make_tool("web_search", active=True)
|
|
second.description = "Second web search"
|
|
|
|
toolset.add_tool(first)
|
|
toolset.add_tool(second)
|
|
|
|
assert len(toolset.tools) == 1
|
|
# The second tool should be the one kept
|
|
assert toolset.tools[0] is second
|
|
assert toolset.tools[0].description == "Second web search"
|
|
|
|
def test_both_inactive_last_one_wins(self):
|
|
"""When both tools are inactive, the new one should overwrite."""
|
|
toolset = ToolSet()
|
|
toolset.add_tool(make_tool("web_search", active=False))
|
|
toolset.add_tool(make_tool("web_search", active=False))
|
|
|
|
assert len(toolset.tools) == 1
|
|
|
|
def test_different_names_both_added(self):
|
|
"""Tools with different names should both be added."""
|
|
toolset = ToolSet()
|
|
toolset.add_tool(make_tool("web_search"))
|
|
toolset.add_tool(make_tool("code_search"))
|
|
|
|
assert len(toolset.tools) == 2
|
|
|
|
def test_missing_active_attribute_defaults_to_true(self):
|
|
"""Tools without 'active' attribute should be treated as active."""
|
|
toolset = ToolSet()
|
|
|
|
# Create a mock object without 'active' attribute
|
|
class MockTool:
|
|
name = "mock_tool"
|
|
description = "Mock"
|
|
parameters = {"type": "object"}
|
|
|
|
mock_tool = MockTool()
|
|
toolset.add_tool(mock_tool) # type: ignore
|
|
|
|
# Should be added successfully
|
|
assert len(toolset.tools) == 1
|
|
|
|
# Adding another tool without active should overwrite
|
|
mock_tool2 = MockTool()
|
|
toolset.add_tool(mock_tool2) # type: ignore
|
|
|
|
assert len(toolset.tools) == 1
|
|
|
|
|
|
class TestFunctionToolManagerGetFunc:
|
|
"""Tests for FunctionToolManager.get_func with conflict resolution."""
|
|
|
|
def test_returns_last_active_tool(self):
|
|
"""Should return the last active tool when multiple have same name."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=True),
|
|
make_tool("web_search", active=True),
|
|
]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
# Should return the last one (reversed order)
|
|
assert result is manager.func_list[1]
|
|
|
|
def test_returns_active_over_inactive(self):
|
|
"""Should prefer active tool over inactive tool with same name."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=False),
|
|
make_tool("web_search", active=True),
|
|
]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
assert result.active is True
|
|
assert result is manager.func_list[1]
|
|
|
|
def test_inactive_cannot_override_active(self):
|
|
"""Inactive tool after active should not be returned."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=True),
|
|
make_tool("web_search", active=False),
|
|
]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
assert result.active is True
|
|
assert result is manager.func_list[0]
|
|
|
|
def test_fallback_to_last_when_none_active(self):
|
|
"""Should return last tool with matching name when none are active."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=False),
|
|
make_tool("web_search", active=False),
|
|
]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
# Should return the last one (reversed order in fallback)
|
|
assert result is manager.func_list[1]
|
|
|
|
def test_returns_none_when_not_found(self):
|
|
"""Should return None when tool name not found."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [make_tool("other_tool")]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is None
|
|
|
|
|
|
class TestFunctionToolManagerGetFullToolSet:
|
|
"""Tests for FunctionToolManager.get_full_tool_set."""
|
|
|
|
def test_deduplicates_by_name_using_add_tool(self):
|
|
"""Should deduplicate tools using add_tool logic."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=False),
|
|
make_tool("web_search", active=True),
|
|
make_tool("code_search", active=True),
|
|
]
|
|
|
|
toolset = manager.get_full_tool_set()
|
|
|
|
# Should have 2 tools after deduplication
|
|
assert len(toolset.tools) == 2
|
|
# web_search should be active (the MCP version)
|
|
web_search = toolset.get_tool("web_search")
|
|
assert web_search is not None
|
|
assert web_search.active is True
|
|
|
|
def test_wrapping_preserves_tool_name_and_description(self):
|
|
"""_PermissionGuardedTool wrapping should preserve name and description."""
|
|
from astrbot.core.provider.func_tool_manager import _PermissionGuardedTool
|
|
|
|
manager = FunctionToolManager()
|
|
tool = make_tool("web_search")
|
|
manager.func_list = [tool]
|
|
|
|
toolset = manager.get_full_tool_set()
|
|
|
|
wrapped = toolset.tools[0]
|
|
assert wrapped.name == "web_search"
|
|
assert wrapped.description == "Test tool web_search"
|
|
assert isinstance(wrapped, _PermissionGuardedTool)
|
|
|
|
def test_mcp_tool_overrides_disabled_builtin(self):
|
|
"""
|
|
Integration test: MCP tool should override disabled built-in tool.
|
|
This is the core scenario for issue #5821.
|
|
"""
|
|
manager = FunctionToolManager()
|
|
# Simulate: built-in tool registered first (disabled)
|
|
# Then MCP tool registered (enabled)
|
|
manager.func_list = [
|
|
make_tool("web_search", active=False), # Built-in, disabled
|
|
make_tool("web_search", active=True), # MCP, enabled
|
|
]
|
|
|
|
# get_func should return the MCP tool (active one)
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
assert result.active is True
|
|
assert result is manager.func_list[1]
|
|
|
|
# get_full_tool_set should also keep the MCP tool
|
|
toolset = manager.get_full_tool_set()
|
|
assert len(toolset.tools) == 1
|
|
assert toolset.tools[0].active is True
|
|
|
|
def test_disabled_mcp_cannot_override_enabled_builtin(self):
|
|
"""Disabled MCP tool should not override enabled built-in tool."""
|
|
manager = FunctionToolManager()
|
|
manager.func_list = [
|
|
make_tool("web_search", active=True), # Built-in, enabled
|
|
make_tool("web_search", active=False), # MCP, disabled
|
|
]
|
|
|
|
result = manager.get_func("web_search")
|
|
assert result is not None
|
|
assert result.active is True
|
|
assert result is manager.func_list[0]
|
|
|
|
toolset = manager.get_full_tool_set()
|
|
assert len(toolset.tools) == 1
|
|
assert toolset.tools[0].active is True
|