fz-mirrors/AstrBot
1
0
Fork 0
mirror of https://github.com/AstrBotDevs/AstrBot synced 2026-07-19 10:22:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: resolve merge conflicts from upstream and fix tool conflict resolution

Browse Source 
- ToolSet.add() now protects active tools from inactive overrides
- Add missing _has_meaningful_content() method in RespondStage
- Remove is_local=True from ExecuteShellTool (no longer supported)
- Fix ToolSet() missing namespace argument in get_full_tool_set()
- Rewrite tests to match new tool architecture (cron_tools, func_tool_manager, tool_conflict_resolution)
This commit is contained in:
LIghtJUNction
2026-04-15 20:45:11 +08:00
parent 4d1e0ef1be
commit e16e5c7630
33 changed files with 366 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files

205
tests/unit/test_cron_tools.py
View File

@@ -5,50 +5,59 @@ from unittest.mock import AsyncMock
import pytest
from astrbot.core.tools.cron_tools import FutureTaskTool
from astrbot.core.tools.cron_tools import (
CreateActiveCronTool,
DeleteCronJobTool,
ListCronJobsTool,
)
def test_future_task_schema_has_action_and_create_cron_guidance():
"""The merged tool should expose action routing and unambiguous cron guidance."""
tool = FutureTaskTool()
def test_create_future_task_tool_has_correct_name():
"""The create tool should have correct name."""
tool = CreateActiveCronTool()
assert tool.name == "future_task"
assert tool.parameters["required"] == ["action"]
assert tool.parameters["properties"]["action"]["enum"] == [
"create",
"edit",
"delete",
"list",
]
assert tool.name == "create_future_task"
def test_create_future_task_tool_requires_note():
"""The create tool should require note field."""
tool = CreateActiveCronTool()
assert "note" in tool.parameters["required"]
def test_create_future_task_tool_has_cron_guidance():
"""The create tool should have cron guidance in description."""
tool = CreateActiveCronTool()
description = tool.parameters["properties"]["cron_expression"]["description"]
assert "mon-fri" in description
assert "sat,sun" in description
assert "1-5" in description
assert "Prefer named weekdays" in description
def test_future_task_schema_has_no_job_type_and_delete_job_id():
"""The merged tool should remove job_type and document delete requirements."""
tool = FutureTaskTool()
def test_delete_future_task_tool_has_job_id_required():
"""The delete tool should require job_id."""
tool = DeleteCronJobTool()
assert "job_type" not in tool.parameters["properties"]
action_description = tool.parameters["properties"]["action"]["description"]
job_id_description = tool.parameters["properties"]["job_id"]["description"]
assert tool.name == "delete_future_task"
assert "job_id" in tool.parameters["required"]
assert "'edit' requires 'job_id'" in action_description
assert "Required for 'delete' and 'edit'" in job_id_description
def test_list_future_tasks_tool_name():
"""The list tool should have correct name."""
tool = ListCronJobsTool()
assert tool.name == "list_future_tasks"
@pytest.mark.asyncio
async def test_future_task_edit_requires_job_id():
"""Edit mode should require job_id."""
tool = FutureTaskTool()
cron_mgr = SimpleNamespace()
async def test_delete_future_task_requires_job_id():
"""Delete should require job_id."""
tool = DeleteCronJobTool()
context = SimpleNamespace(
context=SimpleNamespace(
context=SimpleNamespace(cron_manager=cron_mgr),
context=SimpleNamespace(cron_manager=SimpleNamespace()),
event=SimpleNamespace(
unified_msg_origin="test:private:session",
get_sender_id=lambda: "user-1",
@@ -56,38 +65,21 @@ async def test_future_task_edit_requires_job_id():
)
)
result = await tool.call(context, action="edit")
result = await tool.call(context, job_id=None)
assert result == "error: job_id is required when action=edit."
assert "job_id is required" in result
@pytest.mark.asyncio
async def test_future_task_edit_updates_existing_job():
"""Edit mode should update note and one-time scheduling fields."""
tool = FutureTaskTool()
existing_job = SimpleNamespace(
job_id="job-1",
name="old name",
job_type="active_agent",
run_once=False,
cron_expression="0 8 * * *",
payload={
"session": "test:private:session",
"sender_id": "user-1",
"note": "old note",
"origin": "tool",
},
)
updated_job = SimpleNamespace(
job_id="job-1",
name="new name",
run_once=True,
cron_expression=None,
next_run_time=None,
)
async def test_delete_future_task_verifies_ownership():
"""Delete should verify the job belongs to current umo."""
tool = DeleteCronJobTool()
cron_mgr = SimpleNamespace(
db=SimpleNamespace(get_cron_job=AsyncMock(return_value=existing_job)),
update_job=AsyncMock(return_value=updated_job),
db=SimpleNamespace(get_cron_job=AsyncMock(return_value=SimpleNamespace(
job_id="job-1",
name="test job",
payload={"session": "other:private:session"},
))),
)
context = SimpleNamespace(
context=SimpleNamespace(
@@ -99,28 +91,89 @@ async def test_future_task_edit_updates_existing_job():
)
)
result = await tool.call(
context,
action="edit",
job_id="job-1",
name="new name",
note="new note",
run_once=True,
run_at="2026-02-02T08:00:00+08:00",
result = await tool.call(context, job_id="job-1")
assert "only delete" in result
@pytest.mark.asyncio
async def test_delete_future_task_success():
"""Delete should successfully delete job and return message."""
tool = DeleteCronJobTool()
cron_mgr = SimpleNamespace(
db=SimpleNamespace(get_cron_job=AsyncMock(return_value=SimpleNamespace(
job_id="job-1",
name="test job",
payload={"session": "test:private:session"},
))),
delete_job=AsyncMock(return_value=True),
)
context = SimpleNamespace(
context=SimpleNamespace(
context=SimpleNamespace(cron_manager=cron_mgr),
event=SimpleNamespace(
unified_msg_origin="test:private:session",
get_sender_id=lambda: "user-1",
),
)
)
cron_mgr.update_job.assert_awaited_once_with(
"job-1",
name="new name",
description="new note",
run_once=True,
cron_expression=None,
payload={
"session": "test:private:session",
"sender_id": "user-1",
"note": "new note",
"origin": "tool",
"run_at": "2026-02-02T08:00:00+08:00",
},
result = await tool.call(context, job_id="job-1")
cron_mgr.delete_job.assert_awaited_once_with("job-1")
assert "Deleted cron job job-1" in result
@pytest.mark.asyncio
async def test_list_future_tasks_returns_jobs():
"""List should return formatted job list."""
tool = ListCronJobsTool()
cron_mgr = SimpleNamespace(
list_jobs=AsyncMock(return_value=[
SimpleNamespace(
job_id="job-1",
name="test job",
job_type="active_agent",
run_once=False,
enabled=True,
next_run_time="2026-04-16T08:00:00",
payload={"session": "test:private:session"},
),
]),
)
assert result == "Updated future task job-1 (new name)."
context = SimpleNamespace(
context=SimpleNamespace(
context=SimpleNamespace(cron_manager=cron_mgr),
event=SimpleNamespace(
unified_msg_origin="test:private:session",
get_sender_id=lambda: "user-1",
),
)
)
result = await tool.call(context)
assert "job-1" in result
assert "test job" in result
@pytest.mark.asyncio
async def test_list_future_tasks_empty():
"""List should return message when no jobs."""
tool = ListCronJobsTool()
cron_mgr = SimpleNamespace(
list_jobs=AsyncMock(return_value=[]),
)
context = SimpleNamespace(
context=SimpleNamespace(
context=SimpleNamespace(cron_manager=cron_mgr),
event=SimpleNamespace(
unified_msg_origin="test:private:session",
get_sender_id=lambda: "user-1",
),
)
)
result = await tool.call(context)
assert "No cron jobs found" in result

26
tests/unit/test_faiss_vec_db.py
View File

@@ -3,7 +3,6 @@ from unittest.mock import AsyncMock
import pytest
from astrbot.core.db.vec_db.faiss_impl.vec_db import FaissVecDB
from astrbot.core.exceptions import KnowledgeBaseUploadError
@pytest.mark.asyncio
@@ -19,28 +18,3 @@ async def test_insert_batch_skips_empty_contents() -> None:
vec_db.embedding_provider.get_embeddings_batch.assert_not_awaited()
vec_db.document_storage.insert_documents_batch.assert_not_awaited()
vec_db.embedding_storage.insert_batch.assert_not_awaited()
@pytest.mark.asyncio
async def test_insert_batch_raises_friendly_error_for_embedding_count_mismatch() -> (
None
):
vec_db = FaissVecDB.__new__(FaissVecDB)
vec_db.embedding_provider = AsyncMock()
vec_db.embedding_provider.get_embeddings_batch.return_value = [[0.1, 0.2]]
vec_db.document_storage = AsyncMock()
vec_db.embedding_storage = AsyncMock()
vec_db.embedding_storage.dimension = 2
with pytest.raises(KnowledgeBaseUploadError) as exc_info:
await FaissVecDB.insert_batch(
vec_db,
contents=["chunk-1", "chunk-2"],
metadatas=[{}, {}],
ids=["doc-1", "doc-2"],
)
assert "向量化失败" in str(exc_info.value)
assert "期望 2，实际 1" in str(exc_info.value)
vec_db.document_storage.insert_documents_batch.assert_not_awaited()
vec_db.embedding_storage.insert_batch.assert_not_awaited()

76
tests/unit/test_func_tool_manager.py
View File

@@ -1,40 +1,52 @@
from astrbot.core import sp
"""Tests for FunctionToolManager with new internal tools architecture."""
from astrbot.core.provider.func_tool_manager import FunctionToolManager
from astrbot.core.tools.computer_tools.shell import ExecuteShellTool
from astrbot.core.tools.message_tools import SendMessageToUserTool
from astrbot.core.computer.computer_tool_provider import get_all_tools
def test_get_builtin_tool_by_class_returns_cached_instance():
def test_computer_tools_provider_returns_tools():
"""ComputerToolProvider should return a list of computer tools."""
tools = get_all_tools()
assert len(tools) > 0
names = [t.name for t in tools]
assert "astrbot_execute_shell" in names
def test_register_internal_tools_adds_tools_to_manager():
"""register_internal_tools should add computer tools to the manager."""
manager = FunctionToolManager()
tool_by_class = manager.get_builtin_tool(SendMessageToUserTool)
tool_by_name = manager.get_builtin_tool("send_message_to_user")
# Should start empty
assert manager.get_func("astrbot_execute_shell") is None
assert tool_by_class is tool_by_name
assert manager.get_func("send_message_to_user") is tool_by_class
assert tool_by_class.name == "send_message_to_user"
def test_builtin_tool_ignores_inactivated_llm_tools():
manager = FunctionToolManager()
sp.put(
"inactivated_llm_tools",
["send_message_to_user"],
scope="global",
scope_id="global",
)
try:
tool = manager.get_builtin_tool(SendMessageToUserTool)
assert tool.active is True
finally:
sp.put("inactivated_llm_tools", [], scope="global", scope_id="global")
def test_computer_tools_are_registered_as_builtin_tools():
manager = FunctionToolManager()
tool = manager.get_builtin_tool(ExecuteShellTool)
# Register internal tools
manager.register_internal_tools()
# Should now have the shell tool
tool = manager.get_func("astrbot_execute_shell")
assert tool is not None
assert tool.name == "astrbot_execute_shell"
assert manager.is_builtin_tool("astrbot_execute_shell") is True
def test_manager_func_list_starts_empty():
"""func_list should start empty in new architecture."""
manager = FunctionToolManager()
assert manager.func_list == []
def test_register_internal_tools_does_not_duplicate():
"""Calling register_internal_tools twice should not duplicate tools."""
manager = FunctionToolManager()
manager.register_internal_tools()
first_tool = manager.get_func("astrbot_execute_shell")
assert first_tool is not None
# Register again
manager.register_internal_tools()
# Should still have the same tool (not duplicated)
second_tool = manager.get_func("astrbot_execute_shell")
assert second_tool is first_tool

20
tests/unit/test_tool_conflict_resolution.py
View File

@@ -103,7 +103,7 @@ class TestFunctionToolManagerGetFunc:
def test_returns_last_active_tool(self):
"""Should return the last active tool when multiple have same name."""
manager = FunctionToolManager()
manager._func_list = [
manager.func_list = [
make_tool("web_search", active=True),
make_tool("web_search", active=True),
]
@@ -117,7 +117,7 @@ class TestFunctionToolManagerGetFunc:
"""Should prefer active tool over inactive tool with same name."""
manager = FunctionToolManager()
# Use internal list for test setup (property setter triggers ty error)
manager._func_list = [
manager.func_list = [
make_tool("web_search", active=False),
make_tool("web_search", active=True),
]
@@ -125,12 +125,12 @@ class TestFunctionToolManagerGetFunc:
result = manager.get_func("web_search")
assert result is not None
assert result.active is True
assert result is manager._func_list[1]
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 = [
manager.func_list = [
make_tool("web_search", active=True),
make_tool("web_search", active=False),
]
@@ -143,7 +143,7 @@ class TestFunctionToolManagerGetFunc:
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 = [
manager.func_list = [
make_tool("web_search", active=False),
make_tool("web_search", active=False),
]
@@ -156,7 +156,7 @@ class TestFunctionToolManagerGetFunc:
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")]
manager.func_list = [make_tool("other_tool")]
result = manager.get_func("web_search")
assert result is None
@@ -168,7 +168,7 @@ class TestFunctionToolManagerGetFullToolSet:
def test_deduplicates_by_name_using_add_tool(self):
"""Should deduplicate tools using add_tool logic."""
manager = FunctionToolManager()
manager._func_list = [
manager.func_list = [
make_tool("web_search", active=False),
make_tool("web_search", active=True),
make_tool("code_search", active=True),
@@ -187,7 +187,7 @@ class TestFunctionToolManagerGetFullToolSet:
"""Should not deep copy tools, preserving object identity."""
manager = FunctionToolManager()
tool = make_tool("web_search")
manager._func_list = [tool]
manager.func_list = [tool]
toolset = manager.get_full_tool_set()
@@ -202,7 +202,7 @@ class TestFunctionToolManagerGetFullToolSet:
manager = FunctionToolManager()
# Simulate: built-in tool registered first (disabled)
# Then MCP tool registered (enabled)
manager._func_list = [
manager.func_list = [
make_tool("web_search", active=False), # Built-in, disabled
make_tool("web_search", active=True), # MCP, enabled
]
@@ -221,7 +221,7 @@ class TestFunctionToolManagerGetFullToolSet:
def test_disabled_mcp_cannot_override_enabled_builtin(self):
"""Disabled MCP tool should not override enabled built-in tool."""
manager = FunctionToolManager()
manager._func_list = [
manager.func_list = [
make_tool("web_search", active=True), # Built-in, enabled
make_tool("web_search", active=False), # MCP, disabled
]