fz-mirrors/AstrBot
1
0
Fork 0
mirror of https://github.com/AstrBotDevs/AstrBot synced 2026-07-16 01:40:15 +08:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'master' into dev

Browse Source
This commit is contained in:
LIghtJUNction
2026-03-21 00:31:18 +08:00
parent c238880cd2 d2e0bc778a
commit 1b92c517e7
10 changed files with 427 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
astrbot/core/agent/mcp_client.py
View File

@@ -1,5 +1,7 @@
import asyncio
import logging
import os
import sys
from contextlib import AsyncExitStack
from datetime import timedelta
from typing import Generic
@@ -45,6 +47,22 @@ def _prepare_config(config: dict) -> dict:
return config
def _prepare_stdio_env(config: dict) -> dict:
"""Preserve Windows executable resolution for stdio subprocesses."""
if sys.platform != "win32":
return config
pathext = os.environ.get("PATHEXT")
if not pathext:
return config
prepared = config.copy()
env = dict(prepared.get("env") or {})
env.setdefault("PATHEXT", pathext)
prepared["env"] = env
return prepared
async def _quick_test_mcp_connection(config: dict) -> tuple[bool, str]:
"""Quick test MCP server connectivity"""
import aiohttp
@@ -214,6 +232,7 @@ class MCPClient:
)
else:
cfg = _prepare_stdio_env(cfg)
server_params = mcp.StdioServerParameters(
**cfg,
)

16
astrbot/core/agent/tool.py
View File

@@ -94,11 +94,21 @@ class ToolSet:
return len(self.tools) == 0
def add_tool(self, tool: FunctionTool) -> None:
"""Add a tool to the set."""
# 检查是否已存在同名工具
"""Add a tool to the set.
If a tool with the same name already exists:
- Prefer the one that is active (active=True)
- If both have the same active state, use the new one (overwrite)
"""
for i, existing_tool in enumerate(self.tools):
if existing_tool.name == tool.name:
self.tools[i] = tool
# Use getattr with a default of True for compatibility with tools
# that may not define an `active` attribute (e.g., mocks).
existing_active = bool(getattr(existing_tool, "active", True))
new_active = bool(getattr(tool, "active", True))
# Overwrite if new tool is active, or if existing tool is not active
if new_active or not existing_active:
self.tools[i] = tool
return
self.tools.append(tool)

8
astrbot/core/config/default.py
View File

@@ -1091,7 +1091,7 @@ CONFIG_METADATA_2 = {
"type": "list",
# provider sources templates
"config_template": {
"OpenAI": {
"OpenAI Compatible": {
"id": "openai",
"provider": "openai",
"type": "openai_chat_completion",
@@ -1271,6 +1271,7 @@ CONFIG_METADATA_2 = {
"api_base": "http://127.0.0.1:11434/v1",
"proxy": "",
"custom_headers": {},
"ollama_disable_thinking": False,
},
"LM Studio": {
"id": "lm_studio",
@@ -1831,6 +1832,11 @@ CONFIG_METADATA_2 = {
"items": {},
"hint": "此处添加的键值对将被合并到 OpenAI SDK 的 default_headers 中,用于自定义 HTTP 请求头｡值必须为字符串｡",
},
"ollama_disable_thinking": {
"description": "关闭思考模式",
"type": "bool",
"hint": "关闭 Ollama 思考模式。",
},
"custom_extra_body": {
"description": "自定义请求体参数",
"type": "dict",

23
astrbot/core/provider/func_tool_manager.py
View File

@@ -313,13 +313,30 @@ class FunctionToolManager:
break
def get_func(self, name) -> FuncTool | None:
for f in self.func_list:
# 优先返回已激活的工具（后加载的覆盖前面的，与 ToolSet.add_tool 保持一致）
# 使用 getattr(..., True) 与 ToolSet.add_tool 保持一致：没有 active 属性的工具视为已激活
for f in reversed(self.func_list):
if f.name == name and getattr(f, "active", True):
return f
# 退化则拿最后一个同名工具
for f in reversed(self.func_list):
if f.name == name:
return f
return None
def get_full_tool_set(self) -> ToolSet:
"""获取完整工具集"""
tool_set = ToolSet(self.func_list.copy())
"""获取完整工具集
使用 ToolSet.add_tool 进行填充。对于同名工具，去重规则为：
- 优先保留 active=True 的工具；
- 当 active 状态相同时，后加载的工具会覆盖前面的工具。
因此，后加载的 inactive 工具不会覆盖已激活的工具；
同时，MCP 工具在需要时仍可覆盖被禁用的内置工具。
"""
tool_set = ToolSet()
for tool in self.func_list:
tool_set.add_tool(tool)
return tool_set
@staticmethod

22
astrbot/core/provider/sources/openai_source.py
View File

@@ -211,6 +211,26 @@ class ProviderOpenAIOfficial(Provider):
self.reasoning_key = "reasoning_content"
def _ollama_disable_thinking_enabled(self) -> bool:
value = self.provider_config.get("ollama_disable_thinking", False)
if isinstance(value, str):
return value.strip().lower() in {"1", "true", "yes", "on"}
return bool(value)
def _apply_provider_specific_extra_body_overrides(
self, extra_body: dict[str, Any]
) -> None:
if self.provider_config.get("provider") != "ollama":
return
if not self._ollama_disable_thinking_enabled():
return
# Ollama's OpenAI-compatible endpoint reliably maps reasoning_effort=none
# to think=false, while direct think=false passthrough is not stable.
extra_body.pop("reasoning", None)
extra_body.pop("think", None)
extra_body["reasoning_effort"] = "none"
async def get_models(self):
try:
models_str = []
@@ -246,6 +266,7 @@ class ProviderOpenAIOfficial(Provider):
custom_extra_body = self.provider_config.get("custom_extra_body", {})
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
self._apply_provider_specific_extra_body_overrides(extra_body)
model = payloads.get("model", "").lower()
@@ -296,6 +317,7 @@ class ProviderOpenAIOfficial(Provider):
to_del.append(key)
for key in to_del:
del payloads[key]
self._apply_provider_specific_extra_body_overrides(extra_body)
stream = await self.client.chat.completions.create(
**payloads,

25
dashboard/src/composables/useProviderSources.ts
View File

@@ -204,11 +204,10 @@ export function useProviderSources(options: UseProviderSourcesOptions) {
const advancedSourceConfig = computed(() => {
if (!editableProviderSource.value) return null
const excluded = ['id', 'key', 'api_base', 'enable', 'type', 'provider_type', 'provider']
const excluded = new Set(['id', 'key', 'api_base', 'enable', 'type', 'provider_type', 'provider'])
const advanced: Record<string, any> = {}
for (const key of Object.keys(editableProviderSource.value)) {
if (excluded.includes(key)) continue
Object.defineProperty(advanced, key, {
get() {
return editableProviderSource.value![key]
@@ -216,7 +215,7 @@ export function useProviderSources(options: UseProviderSourcesOptions) {
set(val) {
editableProviderSource.value![key] = val
},
enumerable: true
enumerable: !excluded.has(key)
})
}
@@ -347,7 +346,9 @@ export function useProviderSources(options: UseProviderSourcesOptions) {
selectedProviderSource.value = source
selectedProviderSourceOriginalId.value = source?.id || null
suppressSourceWatch = true
editableProviderSource.value = source ? JSON.parse(JSON.stringify(source)) : null
editableProviderSource.value = source
? ensureProviderSourceDefaults(JSON.parse(JSON.stringify(source)))
: null
nextTick(() => {
suppressSourceWatch = false
})
@@ -356,6 +357,18 @@ export function useProviderSources(options: UseProviderSourcesOptions) {
isSourceModified.value = false
}
function ensureProviderSourceDefaults(source: any) {
if (!source || typeof source !== 'object') {
return source
}
if (source.provider === 'ollama' && source.ollama_disable_thinking === undefined) {
source.ollama_disable_thinking = false
}
return source
}
function extractSourceFieldsFromTemplate(template: Record<string, any>) {
const sourceFields: Record<string, any> = {}
const excludeKeys = ['id', 'enable', 'model', 'provider_source_id', 'modalities', 'custom_extra_body']
@@ -391,14 +404,14 @@ export function useProviderSources(options: UseProviderSourcesOptions) {
}
const newId = generateUniqueSourceId(template.id)
const newSource = {
const newSource = ensureProviderSourceDefaults({
...extractSourceFieldsFromTemplate(template),
id: newId,
type: template.type,
provider_type: template.provider_type,
provider: template.provider,
enable: true
}
})
providerSources.value.push(newSource)
selectedProviderSource.value = newSource

4
dashboard/src/i18n/locales/en-US/features/config-metadata.json
View File

@@ -1069,6 +1069,10 @@
"description": "Custom request headers",
"hint": "Key/value pairs added here are merged into the OpenAI SDK default_headers for custom HTTP headers. Values must be strings."
},
"ollama_disable_thinking": {
"description": "Disable thinking mode",
"hint": "Close Ollama thinking mode."
},
"custom_extra_body": {
"description": "Custom request body parameters",
"hint": "Add extra parameters to requests, such as temperature, top_p, max_tokens, etc.",

4
dashboard/src/i18n/locales/zh-CN/features/config-metadata.json
View File

@@ -1071,6 +1071,10 @@
"description": "自定义添加请求头",
"hint": "此处添加的键值对将被合并到 OpenAI SDK 的 default_headers 中，用于自定义 HTTP 请求头。值必须为字符串。"
},
"ollama_disable_thinking": {
"description": "关闭思考模式",
"hint": "关闭 Ollama 思考模式。"
},
"custom_extra_body": {
"description": "自定义请求体参数",
"hint": "用于在请求时添加额外的参数，如 temperature、top_p、max_tokens 等。",

86
tests/test_openai_source.py
View File

@@ -1,6 +1,7 @@
from types import SimpleNamespace
import pytest
from openai.types.chat.chat_completion import ChatCompletion
from astrbot.core.provider.sources.groq_source import ProviderGroq
from astrbot.core.provider.sources.openai_source import ProviderOpenAIOfficial
@@ -447,3 +448,88 @@ async def test_handle_api_error_unknown_image_error_raises():
)
finally:
await provider.terminate()
@pytest.mark.asyncio
async def test_apply_provider_specific_extra_body_overrides_disables_ollama_thinking():
provider = _make_provider(
{
"provider": "ollama",
"ollama_disable_thinking": True,
}
)
try:
extra_body = {
"reasoning": {"effort": "high"},
"reasoning_effort": "low",
"think": True,
"temperature": 0.2,
}
provider._apply_provider_specific_extra_body_overrides(extra_body)
assert extra_body["reasoning_effort"] == "none"
assert "reasoning" not in extra_body
assert "think" not in extra_body
assert extra_body["temperature"] == 0.2
finally:
await provider.terminate()
@pytest.mark.asyncio
async def test_query_injects_reasoning_effort_none_for_ollama(monkeypatch):
provider = _make_provider(
{
"provider": "ollama",
"ollama_disable_thinking": True,
"custom_extra_body": {
"reasoning": {"effort": "high"},
"temperature": 0.1,
},
}
)
try:
captured_kwargs = {}
async def fake_create(**kwargs):
captured_kwargs.update(kwargs)
return ChatCompletion.model_validate(
{
"id": "chatcmpl-test",
"object": "chat.completion",
"created": 0,
"model": "qwen3.5:4b",
"choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": "ok",
},
"finish_reason": "stop",
}
],
"usage": {
"prompt_tokens": 1,
"completion_tokens": 1,
"total_tokens": 2,
},
}
)
monkeypatch.setattr(provider.client.chat.completions, "create", fake_create)
await provider._query(
payloads={
"model": "qwen3.5:4b",
"messages": [{"role": "user", "content": "hello"}],
},
tools=None,
)
extra_body = captured_kwargs["extra_body"]
assert extra_body["reasoning_effort"] == "none"
assert "reasoning" not in extra_body
assert extra_body["temperature"] == 0.1
finally:
await provider.terminate()

233
tests/unit/test_tool_conflict_resolution.py Normal file
View File

@@ -0,0 +1,233 @@
"""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_no_deepcopy_preserves_identity(self):
"""Should not deep copy tools, preserving object identity."""
manager = FunctionToolManager()
tool = make_tool("web_search")
manager.func_list = [tool]
toolset = manager.get_full_tool_set()
# Same object reference (no deepcopy)
assert toolset.tools[0] is tool
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