diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 8c91adbbfd..4ba6142a27 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -36,10 +36,14 @@ from astrbot.core.persona_error_reply import ( extract_persona_custom_error_message_from_event, ) +from astrbot.core.provider.entities import ( + MALFORMED_TOOL_NAME_PLACEHOLDER as _MALFORMED_TOOL_NAME_PLACEHOLDER, +) from astrbot.core.provider.entities import ( LLMResponse, ProviderRequest, ToolCallsResult, + fallback_tool_call_id, ) from astrbot.core.provider.modalities import ( log_context_sanitize_stats, @@ -144,7 +148,7 @@ class ToolLoopAgentRunner(BaseAgentRunner[TContext]): REPEATED_TOOL_NOTICE_L1_THRESHOLD = 3 REPEATED_TOOL_NOTICE_L2_THRESHOLD = 4 REPEATED_TOOL_NOTICE_L3_THRESHOLD = 5 - MALFORMED_TOOL_NAME_PLACEHOLDER = "__malformed_tool_name__" + MALFORMED_TOOL_NAME_PLACEHOLDER = _MALFORMED_TOOL_NAME_PLACEHOLDER REPEATED_TOOL_NOTICE_L1_TEMPLATE = ( "\n\n[SYSTEM NOTICE] By the way, you have executed the same tool " "`{tool_name}` with the same arguments {streak} times consecutively. " @@ -771,7 +775,15 @@ def _sanitize_malformed_tool_calls( self, llm_resp: LLMResponse, ) -> None: - """Normalize malformed tool call names. + """Normalize malformed tool call names and ids. + + Some OpenAI-compatible upstreams return tool calls without a usable + ``name`` or ``id`` (refs: AstrBot#8911 / AstrBot#9590). ``ToolCall.id`` and + ``ToolCall.FunctionBody.name`` are both required strings, so leaving them + as ``None`` blows up later when the assistant message is assembled. + Normalizing here — the single funnel every provider response passes + through — guarantees the executed tool result and the assistant tool call + share the very same id, which the OpenAI spec requires. Args: llm_resp: The LLM response whose tool call lists should be sanitized. @@ -783,6 +795,42 @@ def _sanitize_malformed_tool_calls( for tool_name in llm_resp.tools_call_name ] + total = max(len(llm_resp.tools_call_name), len(llm_resp.tools_call_args)) + original_ids = list(llm_resp.tools_call_ids) + ids: list[str | None] = original_ids[:total] + # pad so that zip() in _handle_function_tools never silently truncates + ids.extend([None] * (total - len(ids))) + + seen: set[str] = set() + normalized_ids: list[str] = [] + for idx, call_id in enumerate(ids): + if isinstance(call_id, str) and call_id.strip(): + candidate = call_id + else: + candidate = fallback_tool_call_id(idx) + if candidate in seen: + # extremely unlikely, but a placeholder must never collide with a + # real id, otherwise tool results get paired with the wrong call + candidate = f"{candidate}_{uuid.uuid4().hex[:6]}" + seen.add(candidate) + normalized_ids.append(candidate) + if ( + candidate != call_id + and isinstance(call_id, str) + and call_id in llm_resp.tools_call_extra_content + ): + # keep provider-specific payloads reachable under the new id + # (e.g. gemini thought_signature) + llm_resp.tools_call_extra_content[candidate] = ( + llm_resp.tools_call_extra_content[call_id] + ) + + if normalized_ids != original_ids: + logger.warning( + f"Normalized malformed tool call ids: {original_ids} -> {normalized_ids}" + ) + llm_resp.tools_call_ids = normalized_ids + @override async def step(self): """Process a single step of the agent. diff --git a/astrbot/core/provider/entities.py b/astrbot/core/provider/entities.py index 2fab40ca78..c7826d5dca 100644 --- a/astrbot/core/provider/entities.py +++ b/astrbot/core/provider/entities.py @@ -25,6 +25,26 @@ from astrbot.core.message.message_event_result import MessageChain from astrbot.core.utils.media_utils import MediaResolver +MALFORMED_TOOL_NAME_PLACEHOLDER = "__malformed_tool_name__" +"""Placeholder used when the upstream returns a tool call without a usable name.""" + + +def fallback_tool_call_id(idx: int) -> str: + """Build a deterministic placeholder id for a tool call missing its ``id``. + + Some OpenAI-compatible upstreams (Gemini's compatible endpoint, several + proxy gateways) omit ``tool_calls[].id``. The OpenAI spec still requires the + following ``role="tool"`` message to reference the very same id, so both the + provider layer and the agent runner must derive the same placeholder. + + Args: + idx: The position of the tool call within the current response. + + Returns: + A deterministic placeholder tool call id. + """ + return f"call_{idx}" + class ProviderType(enum.Enum): CHAT_COMPLETION = "chat_completion" @@ -403,23 +423,44 @@ def completion_text(self, value) -> None: else: self._completion_text = value + def _safe_tool_call_id(self, idx: int) -> str: + """Return a valid tool call id for ``idx``. + + Falls back to :func:`fallback_tool_call_id` when the upstream omitted the + id or the id list is shorter than the argument list. + """ + call_id = self.tools_call_ids[idx] if idx < len(self.tools_call_ids) else None + if isinstance(call_id, str) and call_id.strip(): + return call_id + return fallback_tool_call_id(idx) + + def _safe_tool_call_name(self, idx: int) -> str: + """Return a valid tool call name for ``idx``. + + ``ToolCall.FunctionBody.name`` is a required string, so a ``None`` name + would raise the very same validation error as a missing id. + """ + name = self.tools_call_name[idx] if idx < len(self.tools_call_name) else None + if isinstance(name, str) and name.strip(): + return name + return MALFORMED_TOOL_NAME_PLACEHOLDER + @deprecated(reason="Use to_openai_tool_calls_model instead.") def to_openai_tool_calls(self) -> list[dict]: """Convert to OpenAI tool calls format. Deprecated, use to_openai_tool_calls_model instead.""" ret = [] for idx, tool_call_arg in enumerate(self.tools_call_args): + call_id = self._safe_tool_call_id(idx) payload = { - "id": self.tools_call_ids[idx], + "id": call_id, "function": { - "name": self.tools_call_name[idx], + "name": self._safe_tool_call_name(idx), "arguments": json.dumps(tool_call_arg), }, "type": "function", } - if self.tools_call_extra_content.get(self.tools_call_ids[idx]): - payload["extra_content"] = self.tools_call_extra_content[ - self.tools_call_ids[idx] - ] + if self.tools_call_extra_content.get(call_id): + payload["extra_content"] = self.tools_call_extra_content[call_id] ret.append(payload) return ret @@ -427,17 +468,16 @@ def to_openai_tool_calls_model(self) -> list[ToolCall]: """The same as to_openai_tool_calls but return pydantic model.""" ret = [] for idx, tool_call_arg in enumerate(self.tools_call_args): + call_id = self._safe_tool_call_id(idx) ret.append( ToolCall( - id=self.tools_call_ids[idx], + id=call_id, function=ToolCall.FunctionBody( - name=self.tools_call_name[idx], + name=self._safe_tool_call_name(idx), arguments=json.dumps(tool_call_arg), ), # the extra_content will not serialize if it's None when calling ToolCall.model_dump() - extra_content=self.tools_call_extra_content.get( - self.tools_call_ids[idx] - ), + extra_content=self.tools_call_extra_content.get(call_id), ), ) return ret diff --git a/astrbot/core/provider/sources/openai_source.py b/astrbot/core/provider/sources/openai_source.py index f7870b7137..57faab163e 100644 --- a/astrbot/core/provider/sources/openai_source.py +++ b/astrbot/core/provider/sources/openai_source.py @@ -28,7 +28,12 @@ from astrbot.core.agent.tool import ToolSet from astrbot.core.exceptions import EmptyModelOutputError from astrbot.core.message.message_event_result import MessageChain -from astrbot.core.provider.entities import LLMResponse, TokenUsage, ToolCallsResult +from astrbot.core.provider.entities import ( + LLMResponse, + TokenUsage, + ToolCallsResult, + fallback_tool_call_id, +) from astrbot.core.utils.media_utils import ( describe_media_ref, resolve_media_ref_to_base64_data, @@ -637,6 +642,11 @@ async def _query_stream( llm_response = LLMResponse("assistant", is_chunk=True) state = ChatCompletionStreamState() + # 上游返回的 tool_call.index 可能从 1 开始、乱序或缺失，而 openai SDK 直接把它 + # 当成 tool_calls 列表的下标使用（_build_events / accumulate_delta），一旦错位就会 + # insert 出一个只有 arguments、没有 id / name 的幽灵 tool_call（refs: AstrBot#9590）。 + # 这里按出现顺序把它重映射成从 0 开始的连续序号，正常上游是恒等映射。 + tool_call_index_map: dict[int, int] = {} async for chunk in stream: choice = chunk.choices[0] if chunk.choices else None @@ -649,8 +659,18 @@ async def _query_stream( tc.type = "function" # Fix for #6661: Add missing 'index' field to tool_call deltas # Gemini and some OpenAI-compatible proxies omit this field - if not hasattr(tc, "index") or tc.index is None: - tc.index = idx + raw_index = getattr(tc, "index", None) + if raw_index is None: + raw_index = idx + # Fix for #9590: normalize 1-based / non-contiguous indexes + if raw_index not in tool_call_index_map: + tool_call_index_map[raw_index] = len(tool_call_index_map) + normalized_index = tool_call_index_map[raw_index] + if normalized_index != raw_index: + logger.debug( + f"normalize tool_call index {raw_index} -> {normalized_index}" + ) + tc.index = normalized_index # 跳过 delta=None 的 chunk，避免 SDK 内部 _convert_initial_chunk_into_snapshot # 第 747 行 choice.delta.to_dict() 抛出 NoneType 错误。 # refs: AstrBot#6689 / openai-python#5069 / #5047 @@ -907,12 +927,25 @@ async def _parse_openai_completion( args = {} args_ls.append(args) func_name_ls.append(tool_call.function.name) - tool_call_ids.append(tool_call.id) + # Some OpenAI-compatible upstreams omit tool_calls[].id. + # The openai SDK deserializes responses leniently, so the + # required field silently becomes None instead of raising. + raw_call_id = getattr(tool_call, "id", None) + if isinstance(raw_call_id, str) and raw_call_id.strip(): + call_id = raw_call_id + else: + # keep the placeholder aligned with the final list index so + # that the agent runner and the assistant message agree on it + call_id = fallback_tool_call_id(len(tool_call_ids)) + logger.warning( + f"上游未返回 tool_call.id，回退为 {call_id}（tool={tool_call.function.name}）" + ) + tool_call_ids.append(call_id) # gemini-2.5 / gemini-3 series extra_content handling extra_content = getattr(tool_call, "extra_content", None) if extra_content is not None: - tool_call_extra_content_dict[tool_call.id] = extra_content + tool_call_extra_content_dict[call_id] = extra_content llm_response.role = "tool" llm_response.tools_call_args = args_ls diff --git a/tests/test_openai_source.py b/tests/test_openai_source.py index 911b76131f..f931a4dd11 100644 --- a/tests/test_openai_source.py +++ b/tests/test_openai_source.py @@ -11,6 +11,7 @@ import astrbot.core.provider.sources.openai_source as openai_source_module import astrbot.core.provider.sources.request_retry as request_retry +from astrbot.core.agent.tool import ToolSet from astrbot.core.exceptions import EmptyModelOutputError from astrbot.core.provider.entities import LLMResponse from astrbot.core.provider.sources.groq_source import ProviderGroq @@ -2201,3 +2202,161 @@ async def fake_create(**kwargs): assert messages[1] == {"role": "user", "content": "again"} finally: await provider.terminate() + + +def _make_tool_call_chunk(tool_call_delta: dict) -> ChatCompletionChunk: + return ChatCompletionChunk.model_validate( + { + "id": "chatcmpl-toolcall", + "object": "chat.completion.chunk", + "created": 0, + "model": "gpt-4o-mini", + "choices": [ + { + "index": 0, + "delta": {"role": "assistant", "tool_calls": [tool_call_delta]}, + "finish_reason": None, + } + ], + } + ) + + +def _make_finish_chunk(finish_reason: str = "tool_calls") -> ChatCompletionChunk: + return ChatCompletionChunk.model_validate( + { + "id": "chatcmpl-toolcall", + "object": "chat.completion.chunk", + "created": 0, + "model": "gpt-4o-mini", + "choices": [ + {"index": 0, "delta": {}, "finish_reason": finish_reason}, + ], + } + ) + + +async def _collect_stream_responses( + provider: ProviderOpenAIOfficial, + monkeypatch, + chunks: list[ChatCompletionChunk], +) -> list[LLMResponse]: + async def fake_stream(): + for chunk in chunks: + yield chunk + + async def fake_create(**kwargs): + return fake_stream() + + monkeypatch.setattr(provider.client.chat.completions, "create", fake_create) + + return [ + response + async for response in provider._query_stream( + payloads={ + "model": "gpt-4o-mini", + "messages": [{"role": "user", "content": "天气"}], + }, + tools=ToolSet(), + ) + ] + +@pytest.mark.asyncio +async def test_query_stream_normalizes_one_based_tool_call_index(monkeypatch): + """上游 index 从 1 开始时不应产生 id=None 的幽灵 tool_call（refs #9590）。""" + provider = _make_provider() + try: + chunks = [ + _make_tool_call_chunk( + { + "index": 1, + "id": "call_abc", + "type": "function", + "function": {"name": "get_weather", "arguments": ""}, + } + ), + _make_tool_call_chunk( + {"index": 1, "function": {"arguments": '{"city": '}} + ), + _make_tool_call_chunk({"index": 1, "function": {"arguments": '"北京"}'}}), + _make_finish_chunk(), + ] + + responses = await _collect_stream_responses(provider, monkeypatch, chunks) + + final_response = responses[-1] + assert final_response.tools_call_ids == ["call_abc"] + assert final_response.tools_call_name == ["get_weather"] + assert final_response.tools_call_args == [{"city": "北京"}] + tool_calls = final_response.to_openai_tool_calls_model() + assert [tc.id for tc in tool_calls] == ["call_abc"] + finally: + await provider.terminate() + + +@pytest.mark.asyncio +async def test_query_stream_falls_back_when_tool_call_id_missing(monkeypatch): + """上游完全不返回 tool_call.id 时应回退到占位 id 而非 None。""" + provider = _make_provider() + try: + chunks = [ + _make_tool_call_chunk( + { + "index": 0, + "type": "function", + "function": {"name": "get_weather", "arguments": '{"city":'}, + } + ), + _make_tool_call_chunk({"index": 0, "function": {"arguments": '"上海"}'}}), + _make_finish_chunk(), + ] + + responses = await _collect_stream_responses(provider, monkeypatch, chunks) + + final_response = responses[-1] + assert final_response.tools_call_ids == ["call_0"] + assert final_response.tools_call_args == [{"city": "上海"}] + assert final_response.to_openai_tool_calls_model()[0].id == "call_0" + finally: + await provider.terminate() + + +@pytest.mark.asyncio +async def test_parse_openai_completion_falls_back_when_tool_call_id_missing(): + """非流式响应缺 tool_call.id 时不应把 None 带进 ToolCall 模型。""" + provider = _make_provider() + try: + completion = ChatCompletion.model_construct( + id="chatcmpl-noid", + object="chat.completion", + created=0, + model="gemini-3-pro", + choices=[ + { + "index": 0, + "message": { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "type": "function", + "function": { + "name": "get_weather", + "arguments": '{"city": "广州"}', + }, + } + ], + }, + "finish_reason": "tool_calls", + } + ], + ) + + response = await provider._parse_openai_completion(completion, tools=ToolSet()) + + assert response.tools_call_ids == ["call_0"] + assert response.tools_call_name == ["get_weather"] + assert response.tools_call_args == [{"city": "广州"}] + assert response.to_openai_tool_calls_model()[0].id == "call_0" + finally: + await provider.terminate() diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index 1e679de4aa..59b3fd2892 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -2051,6 +2051,149 @@ async def test_follow_up_after_stop_not_merged_into_tool_result( assert ticket_before.resolved.is_set() +class MissingToolCallIdProvider(Provider): + """模拟上游不返回 tool_call.id 的 Provider（refs #9590）。""" + + def __init__(self): + super().__init__({}, {}) + self.call_count = 0 + + def get_current_key(self) -> str: + return "test_key" + + def set_key(self, key: str): + pass + + async def get_models(self) -> list[str]: + return ["test_model"] + + async def text_chat(self, **kwargs) -> LLMResponse: + self.call_count += 1 + if self.call_count == 1: + return LLMResponse( + role="assistant", + completion_text="", + tools_call_name=["test_tool"], + tools_call_args=[{"query": "test"}], + tools_call_ids=[None], # type: ignore[list-item] + usage=TokenUsage(input_other=10, output=5), + ) + return LLMResponse( + role="assistant", + completion_text="这是我的最终回答", + usage=TokenUsage(input_other=10, output=5), + ) + + async def text_chat_stream(self, **kwargs): + response = await self.text_chat(**kwargs) + response.is_chunk = True + yield response + response.is_chunk = False + yield response + + +def test_sanitize_malformed_tool_calls_fills_missing_ids(runner): + """缺失/空白的 tool_call id 必须被回退成确定性占位 id。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[None, " "], # type: ignore[list-item] + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_0", "call_1"] + # 关键：不再抛 ValidationError + assert [tc.id for tc in resp.to_openai_tool_calls_model()] == ["call_0", "call_1"] + + +def test_sanitize_malformed_tool_calls_pads_missing_ids(runner): + """id 列表比工具列表短时必须补齐，避免 zip 静默丢工具。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[], + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_0", "call_1"] + + +def test_sanitize_malformed_tool_calls_avoids_id_collision(runner): + """占位 id 与上游真实 id 撞车时必须去重。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[None, "call_0"], # type: ignore[list-item] + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids[0] == "call_0" + assert resp.tools_call_ids[1] != "call_0" + assert resp.tools_call_ids[1].startswith("call_0_") + + +def test_sanitize_malformed_tool_calls_keeps_valid_ids(runner): + """上游给了合法 id 时不得改写。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a"], + tools_call_args=[{"query": "x"}], + tools_call_ids=["call_upstream_abc"], + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_upstream_abc"] + + +@pytest.mark.asyncio +async def test_tool_call_id_none_does_not_break_step( + runner, provider_request, mock_tool_executor, mock_hooks +): + """tool_call.id 为 None 时整轮不应失败，且 assistant/tool 消息 id 必须配对。""" + provider = MissingToolCallIdProvider() + + await runner.reset( + provider=provider, + request=provider_request, + run_context=ContextWrapper(context=None), + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + responses = [] + async for response in runner.step_until_done(3): + responses.append(response) + + assert [r for r in responses if r.type == "err"] == [] + + assistant_messages = [ + msg + for msg in runner.run_context.messages + if msg.role == "assistant" and msg.tool_calls + ] + assert len(assistant_messages) == 1 + tool_calls = assistant_messages[0].tool_calls + assert tool_calls is not None + assert tool_calls[0].id == "call_0" + + tool_messages = [msg for msg in runner.run_context.messages if msg.role == "tool"] + assert tool_messages + # OpenAI 硬约束：tool 结果消息必须引用同一个 tool_call_id + assert tool_messages[0].tool_call_id == tool_calls[0].id + + if __name__ == "__main__": # 运行测试 pytest.main([__file__, "-v"])