mirror of
https://github.com/AstrBotDevs/AstrBot.git
synced 2026-10-16 15:06:04 +08:00
fix(provider): merge custom server tools with registered tools in Anthropic requests (#10437)
* fix(provider): merge custom server tools with registered tools in Anthropic requests
* fix(provider): dedupe tool names and tolerate null custom body
Address review findings on the custom-server-tools merge:
- A custom tool sharing a name with a registered tool (or another
custom tool) produced duplicate definitions that the API rejects.
Merge by name instead; the custom entry wins, matching the override
authority extra_body already had for every other top-level key.
- An explicit null custom_extra_body crashed on .get("tools"). Null
or non-dict values now normalize to an empty body.
* fix(provider): avoid shadowed id in Anthropic streaming tool merge
---------
Co-authored-by: zouyonghe <62183434+zouyonghe@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import base64
|
||||
import builtins
|
||||
import json
|
||||
from collections.abc import AsyncGenerator
|
||||
from typing import Any, Literal
|
||||
@@ -524,7 +525,25 @@ class ProviderAnthropic(Provider):
|
||||
payloads.get("tool_choice", "auto")
|
||||
)
|
||||
|
||||
extra_body = self.provider_config.get("custom_extra_body", {})
|
||||
extra_body = self.provider_config.get("custom_extra_body") or {}
|
||||
if not isinstance(extra_body, dict):
|
||||
# An explicit null or malformed custom body carries nothing to merge.
|
||||
extra_body = {}
|
||||
# Server-side tools declared in the custom body (e.g. web search on
|
||||
# Anthropic-compatible endpoints) must coexist with the registered
|
||||
# function tools: extra_body replaces same-named top-level keys,
|
||||
# which would silently drop them. Tool definitions get the same
|
||||
# override semantics: a custom tool replaces a registered tool of
|
||||
# the same name instead of duplicating it (the API rejects duplicate
|
||||
# tool names).
|
||||
custom_tools = extra_body.get("tools")
|
||||
if isinstance(custom_tools, list) and isinstance(payloads.get("tools"), list):
|
||||
merged = {}
|
||||
for tool in [*payloads["tools"], *custom_tools]:
|
||||
key = tool.get("name") if isinstance(tool, dict) else None
|
||||
merged[key if key else ("_", id(tool))] = tool
|
||||
payloads["tools"] = list(merged.values())
|
||||
extra_body = {k: v for k, v in extra_body.items() if k != "tools"}
|
||||
|
||||
if "max_tokens" not in payloads:
|
||||
payloads["max_tokens"] = 65536
|
||||
@@ -628,7 +647,20 @@ class ProviderAnthropic(Provider):
|
||||
final_tool_calls = []
|
||||
id = None
|
||||
usage = TokenUsage()
|
||||
extra_body = self.provider_config.get("custom_extra_body", {})
|
||||
extra_body = self.provider_config.get("custom_extra_body") or {}
|
||||
if not isinstance(extra_body, dict):
|
||||
extra_body = {}
|
||||
# See _query: custom server-side tools must not replace the
|
||||
# registered function tools, and a name collision overrides
|
||||
# instead of duplicating.
|
||||
custom_tools = extra_body.get("tools")
|
||||
if isinstance(custom_tools, list) and isinstance(payloads.get("tools"), list):
|
||||
merged = {}
|
||||
for tool in [*payloads["tools"], *custom_tools]:
|
||||
key = tool.get("name") if isinstance(tool, dict) else None
|
||||
merged[key if key else ("_", builtins.id(tool))] = tool
|
||||
payloads["tools"] = list(merged.values())
|
||||
extra_body = {k: v for k, v in extra_body.items() if k != "tools"}
|
||||
reasoning_content = ""
|
||||
reasoning_signature = ""
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
from anthropic import BadRequestError
|
||||
from anthropic import _base_client as anthropic_base_client
|
||||
from anthropic.types import MessageDeltaUsage, Usage
|
||||
from anthropic.types import Message, MessageDeltaUsage, TextBlock, Usage
|
||||
|
||||
from astrbot.core.agent.tool import FunctionTool, ToolSet
|
||||
from astrbot.core.provider.entities import TokenUsage
|
||||
@@ -87,6 +89,203 @@ def test_anthropic_update_usage_omitted_fields_are_preserved():
|
||||
assert token_usage.output == 7
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_merges_custom_server_tools_with_registered_tools():
|
||||
provider = ProviderAnthropic(
|
||||
provider_config={
|
||||
"id": "anthropic-test",
|
||||
"type": "anthropic_chat_completion",
|
||||
"model": "claude-test",
|
||||
"key": ["test-key"],
|
||||
"custom_extra_body": {
|
||||
"tools": [{"type": "web_search_20250305", "name": "web_search"}],
|
||||
"custom_flag": True,
|
||||
},
|
||||
},
|
||||
provider_settings={},
|
||||
)
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
class FakeMessages:
|
||||
async def create(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
return Message(
|
||||
id="msg_1",
|
||||
content=[TextBlock(text="ok", type="text")],
|
||||
model="claude-test",
|
||||
role="assistant",
|
||||
stop_reason="end_turn",
|
||||
stop_sequence=None,
|
||||
type="message",
|
||||
usage=Usage(input_tokens=1, output_tokens=1),
|
||||
)
|
||||
|
||||
provider.client = SimpleNamespace(messages=FakeMessages())
|
||||
|
||||
tool_set = ToolSet(
|
||||
[
|
||||
FunctionTool(
|
||||
name="get_time",
|
||||
description="Get the current time.",
|
||||
parameters={"type": "object", "properties": {}},
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
await provider._query({"model": "claude-test", "messages": []}, tool_set)
|
||||
|
||||
# Server-side tools from the custom body must coexist with the
|
||||
# registered function tools instead of replacing them.
|
||||
assert [tool["name"] for tool in captured["tools"]] == ["get_time", "web_search"]
|
||||
assert "tools" not in captured["extra_body"]
|
||||
assert captured["extra_body"]["custom_flag"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_custom_tools_without_registered_tools_pass_through():
|
||||
provider = ProviderAnthropic(
|
||||
provider_config={
|
||||
"id": "anthropic-test",
|
||||
"type": "anthropic_chat_completion",
|
||||
"model": "claude-test",
|
||||
"key": ["test-key"],
|
||||
"custom_extra_body": {
|
||||
"tools": [{"type": "web_search_20250305", "name": "web_search"}],
|
||||
},
|
||||
},
|
||||
provider_settings={},
|
||||
)
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
class FakeMessages:
|
||||
async def create(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
return Message(
|
||||
id="msg_1",
|
||||
content=[TextBlock(text="ok", type="text")],
|
||||
model="claude-test",
|
||||
role="assistant",
|
||||
stop_reason="end_turn",
|
||||
stop_sequence=None,
|
||||
type="message",
|
||||
usage=Usage(input_tokens=1, output_tokens=1),
|
||||
)
|
||||
|
||||
provider.client = SimpleNamespace(messages=FakeMessages())
|
||||
|
||||
await provider._query({"model": "claude-test", "messages": []}, None)
|
||||
|
||||
# No registered tools: the custom body keeps its previous override
|
||||
# semantics and goes out via extra_body untouched.
|
||||
assert "tools" not in captured
|
||||
assert captured["extra_body"]["tools"] == [
|
||||
{"type": "web_search_20250305", "name": "web_search"}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_custom_tool_overrides_registered_tool_with_same_name():
|
||||
provider = ProviderAnthropic(
|
||||
provider_config={
|
||||
"id": "anthropic-test",
|
||||
"type": "anthropic_chat_completion",
|
||||
"model": "claude-test",
|
||||
"key": ["test-key"],
|
||||
"custom_extra_body": {
|
||||
"tools": [
|
||||
{"type": "web_search_20250305", "name": "web_search", "max_uses": 3}
|
||||
],
|
||||
},
|
||||
},
|
||||
provider_settings={},
|
||||
)
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
class FakeMessages:
|
||||
async def create(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
return Message(
|
||||
id="msg_1",
|
||||
content=[TextBlock(text="ok", type="text")],
|
||||
model="claude-test",
|
||||
role="assistant",
|
||||
stop_reason="end_turn",
|
||||
stop_sequence=None,
|
||||
type="message",
|
||||
usage=Usage(input_tokens=1, output_tokens=1),
|
||||
)
|
||||
|
||||
provider.client = SimpleNamespace(messages=FakeMessages())
|
||||
|
||||
tool_set = ToolSet(
|
||||
[
|
||||
FunctionTool(
|
||||
name="web_search",
|
||||
description="Client-side web search.",
|
||||
parameters={"type": "object", "properties": {}},
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
await provider._query({"model": "claude-test", "messages": []}, tool_set)
|
||||
|
||||
# A name collision must not duplicate the tool (the API rejects
|
||||
# duplicates); the custom body keeps its override authority.
|
||||
assert captured["tools"] == [
|
||||
{"type": "web_search_20250305", "name": "web_search", "max_uses": 3}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_null_custom_extra_body_does_not_break_request():
|
||||
provider = ProviderAnthropic(
|
||||
provider_config={
|
||||
"id": "anthropic-test",
|
||||
"type": "anthropic_chat_completion",
|
||||
"model": "claude-test",
|
||||
"key": ["test-key"],
|
||||
"custom_extra_body": None,
|
||||
},
|
||||
provider_settings={},
|
||||
)
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
class FakeMessages:
|
||||
async def create(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
return Message(
|
||||
id="msg_1",
|
||||
content=[TextBlock(text="ok", type="text")],
|
||||
model="claude-test",
|
||||
role="assistant",
|
||||
stop_reason="end_turn",
|
||||
stop_sequence=None,
|
||||
type="message",
|
||||
usage=Usage(input_tokens=1, output_tokens=1),
|
||||
)
|
||||
|
||||
provider.client = SimpleNamespace(messages=FakeMessages())
|
||||
|
||||
tool_set = ToolSet(
|
||||
[
|
||||
FunctionTool(
|
||||
name="get_time",
|
||||
description="Get the current time.",
|
||||
parameters={"type": "object", "properties": {}},
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
await provider._query({"model": "claude-test", "messages": []}, tool_set)
|
||||
|
||||
assert [tool["name"] for tool in captured["tools"]] == ["get_time"]
|
||||
assert captured["extra_body"] == {}
|
||||
|
||||
|
||||
def _tool_use_stream(
|
||||
partial_json_chunks: list[str], start_input: dict | None = None
|
||||
) -> bytes:
|
||||
@@ -196,3 +395,74 @@ async def test_anthropic_stream_keeps_tool_call_with_empty_input(
|
||||
assert response.tools_call_name == ["get_time"]
|
||||
assert response.tools_call_args == [expected_args]
|
||||
assert response.tools_call_ids == ["toolu_01"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("streaming", [False, True], ids=["standard", "stream"])
|
||||
@pytest.mark.parametrize(
|
||||
"custom_tool",
|
||||
[{}, {"name": ""}, {"name": None}, None, "invalid", 0, False],
|
||||
ids=["missing-name", "empty-name", "null-name", "null", "string", "zero", "false"],
|
||||
)
|
||||
async def test_malformed_custom_tools_reach_api_validation(
|
||||
monkeypatch, streaming, custom_tool
|
||||
):
|
||||
captured = []
|
||||
|
||||
def handle(request):
|
||||
captured.append(json.loads(request.content))
|
||||
return sdk_httpx.Response(
|
||||
400,
|
||||
json={
|
||||
"type": "error",
|
||||
"error": {"type": "invalid_request_error", "message": "Invalid tool"},
|
||||
},
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
ProviderAnthropic,
|
||||
"_create_http_client",
|
||||
lambda self, config: sdk_httpx.AsyncClient(
|
||||
transport=sdk_httpx.MockTransport(handle)
|
||||
),
|
||||
)
|
||||
custom_tools = [custom_tool, {"type": "unnamed"}]
|
||||
provider = ProviderAnthropic(
|
||||
{
|
||||
"id": "test",
|
||||
"type": "anthropic_chat_completion",
|
||||
"key": ["test-key"],
|
||||
"model": "claude-test",
|
||||
"api_base": "https://offline.invalid",
|
||||
"custom_extra_body": {"tools": custom_tools},
|
||||
},
|
||||
{},
|
||||
)
|
||||
tools = ToolSet(
|
||||
[
|
||||
FunctionTool(
|
||||
name="get_time",
|
||||
description="Offline tool schema.",
|
||||
parameters={"type": "object", "properties": {}},
|
||||
)
|
||||
]
|
||||
)
|
||||
payload = {"model": "claude-test", "messages": [{"role": "user", "content": "hi"}]}
|
||||
try:
|
||||
for _ in range(2):
|
||||
with pytest.raises(BadRequestError):
|
||||
if streaming:
|
||||
async for _ in provider._query_stream(
|
||||
payload, tools, request_max_retries=1
|
||||
):
|
||||
pass
|
||||
else:
|
||||
await provider._query(payload, tools, request_max_retries=1)
|
||||
finally:
|
||||
await provider.terminate()
|
||||
|
||||
assert len(captured) == 2
|
||||
assert captured[0] == captured[1]
|
||||
assert captured[0]["tools"][0]["name"] == "get_time"
|
||||
assert captured[0]["tools"][1:] == custom_tools
|
||||
assert provider.provider_config["custom_extra_body"]["tools"] == custom_tools
|
||||
|
||||
Reference in New Issue
Block a user