From 8beb863b90534584a5a2654fcd81fbf3c2f7a2d9 Mon Sep 17 00:00:00 2001 From: AnegasakiNene <990126341@qq.com> Date: Sun, 16 Aug 2026 11:57:14 +0800 Subject: [PATCH 1/5] fix: sanitize incomplete tool history before fallback --- astrbot/core/agent/context/manager.py | 10 +++- astrbot/core/agent/context/truncator.py | 30 +++++++++-- tests/agent/test_context_manager.py | 68 ++++++++++++++++++++++++- tests/agent/test_truncator.py | 59 +++++++++++++++++++++ tests/test_tool_loop_agent_runner.py | 65 +++++++++++++++++++++++ 5 files changed, 225 insertions(+), 7 deletions(-) diff --git a/astrbot/core/agent/context/manager.py b/astrbot/core/agent/context/manager.py index 1a11ebff96..f0f7f5bbd2 100644 --- a/astrbot/core/agent/context/manager.py +++ b/astrbot/core/agent/context/manager.py @@ -53,8 +53,14 @@ async def process( Returns: The processed message list. """ + result = messages try: - result = messages + result = self.truncator.fix_messages(messages) + if len(result) != len(messages): + logger.warning( + f"Removed {len(messages) - len(result)} invalid tool history " + "message(s) before context processing." + ) # 1. 基于轮次的截断 (Enforce max turns) if self.config.enforce_max_turns != -1: @@ -78,7 +84,7 @@ async def process( return result except Exception as e: logger.error(f"Error during context processing: {e}", exc_info=True) - return messages + return result async def _run_compression( self, messages: list[Message], prev_tokens: int diff --git a/astrbot/core/agent/context/truncator.py b/astrbot/core/agent/context/truncator.py index 9abf574336..e61b6afee1 100644 --- a/astrbot/core/agent/context/truncator.py +++ b/astrbot/core/agent/context/truncator.py @@ -53,7 +53,8 @@ def fix_messages(self, messages: list[Message]) -> list[Message]: This method ensures that: 1. Each `tool` message is preceded by an `assistant` message containing `tool_calls`. - 2. Each `assistant` message containing `tool_calls` is followed by corresponding ` + 2. Each `assistant` message containing `tool_calls` is followed by exactly one + `tool` message for every tool call ID. This is a requirement of the OpenAI Chat Completions API specification (Gemini enforces this strictly). """ @@ -66,9 +67,30 @@ def fix_messages(self, messages: list[Message]) -> list[Message]: def flush_pending_if_valid() -> None: nonlocal pending_assistant, pending_tools - if pending_assistant is not None and pending_tools: - fixed_messages.append(pending_assistant) - fixed_messages.extend(pending_tools) + if pending_assistant is not None: + expected_ids = [] + for tool_call in pending_assistant.tool_calls or []: + if isinstance(tool_call, dict): + tool_call_id = tool_call.get("id") + else: + tool_call_id = tool_call.id + if not isinstance(tool_call_id, str) or not tool_call_id: + expected_ids = [] + break + expected_ids.append(tool_call_id) + result_ids = [tool.tool_call_id for tool in pending_tools] + if ( + expected_ids + and len(expected_ids) == len(set(expected_ids)) + and len(result_ids) == len(expected_ids) + and all( + isinstance(tool_id, str) and tool_id for tool_id in result_ids + ) + and len(result_ids) == len(set(result_ids)) + and set(result_ids) == set(expected_ids) + ): + fixed_messages.append(pending_assistant) + fixed_messages.extend(pending_tools) pending_assistant = None pending_tools = [] diff --git a/tests/agent/test_context_manager.py b/tests/agent/test_context_manager.py index a596677e9b..91ebfb8c9a 100644 --- a/tests/agent/test_context_manager.py +++ b/tests/agent/test_context_manager.py @@ -412,6 +412,37 @@ async def test_process_with_no_limits(self): assert len(result) == 20 assert result == messages + @pytest.mark.asyncio + async def test_process_fixes_incomplete_tool_history_without_limits(self): + """Provider-facing history is valid even when no size limit is enabled.""" + config = ContextConfig(max_context_tokens=0, enforce_max_turns=-1) + manager = ContextManager(config) + messages = [ + self.create_message("user", "Run both tools"), + Message( + role="assistant", + content="Calling tools", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + ), + Message(role="tool", content="first result", tool_call_id="call_1"), + self.create_message("user", "Continue"), + ] + + result = await manager.process(messages) + + assert result == [messages[0], messages[-1]] + # ==================== Enforce Max Turns Tests ==================== @pytest.mark.asyncio @@ -655,13 +686,48 @@ async def test_error_handling_returns_original_messages(self): # Make compressor raise an exception with patch.object( - manager.compressor, "__call__", side_effect=Exception("Test error") + manager, "_run_compression", side_effect=Exception("Test error") ): result = await manager.process(messages) # Should return original messages despite error assert result == messages + @pytest.mark.asyncio + async def test_error_handling_keeps_tool_history_sanitized(self): + """Compression errors must not restore an invalid tool history block.""" + config = ContextConfig(max_context_tokens=1) + manager = ContextManager(config) + manager.compressor.should_compress = MagicMock(return_value=True) + messages = [ + self.create_message("user", "Run both tools"), + Message( + role="assistant", + content="Calling tools", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + ), + Message(role="tool", content="first result", tool_call_id="call_1"), + self.create_message("user", "Continue"), + ] + + with patch.object( + manager, "_run_compression", side_effect=Exception("Test error") + ): + result = await manager.process(messages) + + assert result == [messages[0], messages[-1]] + @pytest.mark.asyncio async def test_error_handling_logs_exception(self): """Test that errors are logged.""" diff --git a/tests/agent/test_truncator.py b/tests/agent/test_truncator.py index 7dac80f9ce..99bf1ce54a 100644 --- a/tests/agent/test_truncator.py +++ b/tests/agent/test_truncator.py @@ -62,6 +62,65 @@ def test_fix_messages_tool_without_context(self): # Tool message without context should be removed assert len(result) == 0 + def test_fix_messages_keeps_complete_multi_tool_block(self): + """Keep a tool block when every call has exactly one matching result.""" + truncator = ContextTruncator() + messages = [ + self.create_message("user", "Run both tools"), + Message( + role="assistant", + content="Calling tools", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + ), + Message(role="tool", content="second result", tool_call_id="call_2"), + Message(role="tool", content="first result", tool_call_id="call_1"), + self.create_message("assistant", "Done"), + ] + + result = truncator.fix_messages(messages) + + assert result == messages + + def test_fix_messages_drops_incomplete_multi_tool_block(self): + """Drop the whole block when one of multiple tool results is missing.""" + truncator = ContextTruncator() + messages = [ + self.create_message("user", "Run both tools"), + Message( + role="assistant", + content="Calling tools", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + ), + Message(role="tool", content="first result", tool_call_id="call_1"), + self.create_message("user", "Continue"), + ] + + result = truncator.fix_messages(messages) + + assert result == [messages[0], messages[-1]] + # ==================== truncate_by_turns Tests ==================== def test_truncate_by_turns_no_limit(self): diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index 1e679de4aa..727581d75e 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -183,6 +183,16 @@ async def text_chat(self, **kwargs) -> LLMResponse: raise RuntimeError("primary provider failed") +class CapturingFailingProvider(MockFailingProvider): + def __init__(self): + super().__init__() + self.received_contexts = [] + + async def text_chat(self, **kwargs) -> LLMResponse: + self.received_contexts.append(list(kwargs.get("contexts") or [])) + return await super().text_chat(**kwargs) + + class MockErrProvider(MockProvider): async def text_chat(self, **kwargs) -> LLMResponse: self.call_count += 1 @@ -1213,6 +1223,61 @@ async def test_fallback_provider_used_when_primary_raises( assert fallback_provider.call_count == 1 +@pytest.mark.asyncio +async def test_fallback_providers_receive_only_complete_tool_history( + runner, mock_tool_executor, mock_hooks +): + primary_provider = CapturingFailingProvider() + fallback_provider = CapturingProvider(modalities=[]) + request = ProviderRequest( + prompt="Continue", + contexts=[ + {"role": "user", "content": "Run both tools"}, + { + "role": "assistant", + "content": "Calling tools", + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + }, + {"role": "tool", "content": "first result", "tool_call_id": "call_1"}, + ], + ) + + await runner.reset( + provider=primary_provider, + request=request, + run_context=ContextWrapper(context=None), + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + fallback_providers=[fallback_provider], + ) + + async for _ in runner.step_until_done(5): + pass + + for contexts in [ + primary_provider.received_contexts[0], + fallback_provider.received_contexts[0], + ]: + assert all(message.role != "tool" for message in contexts) + assert all(not message.tool_calls for message in contexts) + + final_resp = runner.get_final_llm_resp() + assert final_resp is not None + assert final_resp.completion_text == "final" + + @pytest.mark.asyncio async def test_fallback_provider_used_when_primary_returns_err( runner, provider_request, mock_tool_executor, mock_hooks From 3dbd608ea2af144e246b46ce3e15fc677a308924 Mon Sep 17 00:00:00 2001 From: AnegasakiNene <990126341@qq.com> Date: Sun, 16 Aug 2026 12:13:02 +0800 Subject: [PATCH 2/5] refactor: clarify tool history validation --- astrbot/core/agent/context/manager.py | 2 +- astrbot/core/agent/context/truncator.py | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/astrbot/core/agent/context/manager.py b/astrbot/core/agent/context/manager.py index f0f7f5bbd2..f85e7e4113 100644 --- a/astrbot/core/agent/context/manager.py +++ b/astrbot/core/agent/context/manager.py @@ -57,7 +57,7 @@ async def process( try: result = self.truncator.fix_messages(messages) if len(result) != len(messages): - logger.warning( + logger.debug( f"Removed {len(messages) - len(result)} invalid tool history " "message(s) before context processing." ) diff --git a/astrbot/core/agent/context/truncator.py b/astrbot/core/agent/context/truncator.py index e61b6afee1..cdb94969da 100644 --- a/astrbot/core/agent/context/truncator.py +++ b/astrbot/core/agent/context/truncator.py @@ -79,16 +79,18 @@ def flush_pending_if_valid() -> None: break expected_ids.append(tool_call_id) result_ids = [tool.tool_call_id for tool in pending_tools] - if ( + has_valid_expected_ids = bool(expected_ids) and len( expected_ids - and len(expected_ids) == len(set(expected_ids)) - and len(result_ids) == len(expected_ids) + ) == len(set(expected_ids)) + has_valid_result_ids = ( + len(result_ids) == len(expected_ids) and all( isinstance(tool_id, str) and tool_id for tool_id in result_ids ) and len(result_ids) == len(set(result_ids)) - and set(result_ids) == set(expected_ids) - ): + ) + ids_match = set(result_ids) == set(expected_ids) + if has_valid_expected_ids and has_valid_result_ids and ids_match: fixed_messages.append(pending_assistant) fixed_messages.extend(pending_tools) pending_assistant = None From 643eae73e31492d9b10fff7031979f5c78ba1e51 Mon Sep 17 00:00:00 2001 From: AnegasakiNene <990126341@qq.com> Date: Sun, 16 Aug 2026 12:26:48 +0800 Subject: [PATCH 3/5] fix: preserve multi-yield tool results --- .../agent/runners/tool_loop_agent_runner.py | 10 +++- tests/test_tool_loop_agent_runner.py | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 8c91adbbfd..9488912a7f 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -1096,11 +1096,19 @@ async def _handle_function_tools( logger.info(f"Agent 使用工具: {llm_response.tools_call_name}") def _append_tool_call_result(tool_call_id: str, content: str) -> None: + content = self._merge_follow_up_notice(content) + if ( + tool_call_result_blocks + and tool_call_result_blocks[-1].tool_call_id == tool_call_id + ): + previous = tool_call_result_blocks[-1] + previous.content = f"{previous.content}\n\n{content}" + return tool_call_result_blocks.append( ToolCallMessageSegment( role="tool", tool_call_id=tool_call_id, - content=self._merge_follow_up_notice(content), + content=content, ), ) diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index 727581d75e..b5c404f345 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -99,6 +99,24 @@ async def generator(): return generator() +class MultiYieldToolExecutor: + """Simulate a local tool that yields multiple results for one call.""" + + @classmethod + def execute(cls, tool, run_context, **tool_args): + async def generator(): + from mcp.types import CallToolResult, TextContent + + yield CallToolResult( + content=[TextContent(type="text", text="first streamed result")] + ) + yield CallToolResult( + content=[TextContent(type="text", text="second streamed result")] + ) + + return generator() + + class LargeTextToolExecutor: """模拟返回超长文本的工具执行器""" @@ -621,6 +639,34 @@ async def snapshot_context_manager(messages, trusted_token_usage=0): assert "工具执行结果" in tool_messages[0].content +@pytest.mark.asyncio +async def test_tool_loop_merges_multiple_results_from_one_tool_call( + runner, provider_request, mock_hooks +): + """Multiple executor yields must remain one complete provider tool result.""" + provider = CapturingToolLoopProvider("test_tool") + + await runner.reset( + provider=provider, + request=provider_request, + run_context=ContextWrapper(context=None), + tool_executor=MultiYieldToolExecutor, + agent_hooks=mock_hooks, + streaming=False, + ) + + async for _ in runner.step_until_done(3): + pass + + assert provider.call_count == 2 + second_contexts = provider.received_contexts[1] + tool_messages = [msg for msg in second_contexts if msg.role == "tool"] + assert len(tool_messages) == 1 + assert tool_messages[0].tool_call_id == "call_context_refresh" + assert "first streamed result" in tool_messages[0].content + assert "second streamed result" in tool_messages[0].content + + @pytest.mark.asyncio async def test_normal_completion_without_max_step( runner, mock_provider, provider_request, mock_tool_executor, mock_hooks From e077836a425927e08a4fa368a68d09b114461b48 Mon Sep 17 00:00:00 2001 From: AnegasakiNene <990126341@qq.com> Date: Sun, 16 Aug 2026 12:32:31 +0800 Subject: [PATCH 4/5] fix: recount tokens after history sanitization --- astrbot/core/agent/context/manager.py | 5 ++-- tests/agent/test_context_manager.py | 38 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/astrbot/core/agent/context/manager.py b/astrbot/core/agent/context/manager.py index f85e7e4113..ab8ba4a415 100644 --- a/astrbot/core/agent/context/manager.py +++ b/astrbot/core/agent/context/manager.py @@ -56,7 +56,8 @@ async def process( result = messages try: result = self.truncator.fix_messages(messages) - if len(result) != len(messages): + history_was_sanitized = result != messages + if history_was_sanitized: logger.debug( f"Removed {len(messages) - len(result)} invalid tool history " "message(s) before context processing." @@ -73,7 +74,7 @@ async def process( # 2. 基于 token 的压缩 if self.config.max_context_tokens > 0: total_tokens = self.token_counter.count_tokens( - result, trusted_token_usage + result, 0 if history_was_sanitized else trusted_token_usage ) if self.compressor.should_compress( diff --git a/tests/agent/test_context_manager.py b/tests/agent/test_context_manager.py index 91ebfb8c9a..a8f7348dab 100644 --- a/tests/agent/test_context_manager.py +++ b/tests/agent/test_context_manager.py @@ -577,6 +577,44 @@ async def test_trusted_usage_triggers_compression_before_provider_call(self): mock_compressor.assert_awaited_once_with(messages) assert result == compressed + @pytest.mark.asyncio + async def test_sanitized_history_does_not_reuse_stale_trusted_usage(self): + config = ContextConfig(max_context_tokens=100, truncate_turns=1) + manager = ContextManager(config) + messages = [ + self.create_message("user", "old request"), + Message( + role="assistant", + content="Calling tools", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "first", "arguments": "{}"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "second", "arguments": "{}"}, + }, + ], + ), + Message(role="tool", content="first result", tool_call_id="call_1"), + self.create_message("user", "current request"), + ] + sanitized = [messages[0], messages[-1]] + mock_compressor = AsyncMock() + mock_compressor.should_compress = MagicMock(return_value=False) + manager.compressor = mock_compressor + + result = await manager.process(messages, trusted_token_usage=83) + + first_check = mock_compressor.should_compress.call_args_list[0] + expected_tokens = manager.token_counter.count_tokens(sanitized) + assert first_check.args == (sanitized, expected_tokens, 100) + mock_compressor.assert_not_awaited() + assert result == sanitized + @pytest.mark.asyncio async def test_token_compression_with_zero_max_tokens(self): """Test that compression is skipped when max_context_tokens is 0.""" From 086a5b3a9fdff3eda36bda74b18bfbfe7747ae07 Mon Sep 17 00:00:00 2001 From: AnegasakiNene <990126341@qq.com> Date: Sun, 16 Aug 2026 12:45:03 +0800 Subject: [PATCH 5/5] fix: normalize Gemini fallback tool call ids --- .../agent/runners/tool_loop_agent_runner.py | 2 +- .../core/provider/sources/gemini_source.py | 22 ++++++- tests/test_gemini_source.py | 66 +++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 9488912a7f..08d922cbf0 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -1098,7 +1098,7 @@ async def _handle_function_tools( def _append_tool_call_result(tool_call_id: str, content: str) -> None: content = self._merge_follow_up_notice(content) if ( - tool_call_result_blocks + len(tool_call_result_blocks) > tool_result_blocks_start and tool_call_result_blocks[-1].tool_call_id == tool_call_id ): previous = tool_call_result_blocks[-1] diff --git a/astrbot/core/provider/sources/gemini_source.py b/astrbot/core/provider/sources/gemini_source.py index abf7bb7cf8..3fa757a63f 100644 --- a/astrbot/core/provider/sources/gemini_source.py +++ b/astrbot/core/provider/sources/gemini_source.py @@ -322,6 +322,7 @@ def append_or_extend( contents.append(content_cls(parts=part)) gemini_contents: list[types.Content] = [] + tool_name_by_call_id: dict[str, str] = {} for message in payloads["messages"]: role, content = message["role"], message.get("content") @@ -392,6 +393,11 @@ def append_or_extend( if "tool_calls" in message: for tool in message["tool_calls"]: + tool_call_id = tool.get("id") + if isinstance(tool_call_id, str) and tool_call_id: + tool_name_by_call_id[tool_call_id] = tool["function"][ + "name" + ] part = types.Part.from_function_call( name=tool["function"]["name"], args=json.loads(tool["function"]["arguments"]), @@ -415,7 +421,12 @@ def append_or_extend( append_or_extend(gemini_contents, parts, types.ModelContent) elif role == "tool": - func_name = message.get("name", message["tool_call_id"]) + tool_call_id = message["tool_call_id"] + func_name = ( + message.get("name") + or tool_name_by_call_id.get(tool_call_id) + or tool_call_id + ) part = types.Part.from_function_response( name=func_name, response={ @@ -547,8 +558,13 @@ def _process_content_parts( llm_response.role = "tool" llm_response.tools_call_name.append(part.function_call.name) llm_response.tools_call_args.append(part.function_call.args) - # function_call.id might be None, use name as fallback - tool_call_id = part.function_call.id or part.function_call.name + # function_call.id might be None, use a unique name-based fallback. + base_tool_call_id = part.function_call.id or part.function_call.name + tool_call_id = base_tool_call_id + duplicate_index = 2 + while tool_call_id in llm_response.tools_call_ids: + tool_call_id = f"{base_tool_call_id}__astrbot_{duplicate_index}" + duplicate_index += 1 llm_response.tools_call_ids.append(tool_call_id) # extra_content if part.thought_signature: diff --git a/tests/test_gemini_source.py b/tests/test_gemini_source.py index 9294ea46b2..88aa9a5aa6 100644 --- a/tests/test_gemini_source.py +++ b/tests/test_gemini_source.py @@ -2,6 +2,7 @@ import httpx import pytest +from google.genai import types from astrbot.core.exceptions import EmptyModelOutputError import astrbot.core.provider.sources.request_retry as request_retry @@ -33,6 +34,71 @@ def test_gemini_reasoning_only_output_is_allowed(): ) +def test_gemini_parallel_same_function_fallback_ids_are_unique(): + provider = ProviderGoogleGenAI.__new__(ProviderGoogleGenAI) + candidate = types.Candidate( + content=types.Content( + parts=[ + types.Part.from_function_call(name="weather", args={"city": "A"}), + types.Part.from_function_call(name="weather", args={"city": "B"}), + ] + ), + finish_reason=types.FinishReason.STOP, + ) + llm_response = LLMResponse(role="assistant") + + provider._process_content_parts(candidate, llm_response) + + assert llm_response.tools_call_ids == ["weather", "weather__astrbot_2"] + + +def test_gemini_tool_responses_restore_function_name_from_unique_call_ids(): + provider = ProviderGoogleGenAI.__new__(ProviderGoogleGenAI) + payloads = { + "messages": [ + {"role": "user", "content": "Check both cities"}, + { + "role": "assistant", + "content": "", + "tool_calls": [ + { + "id": "weather", + "type": "function", + "function": { + "name": "weather", + "arguments": '{"city":"A"}', + }, + }, + { + "id": "weather__astrbot_2", + "type": "function", + "function": { + "name": "weather", + "arguments": '{"city":"B"}', + }, + }, + ], + }, + {"role": "tool", "tool_call_id": "weather", "content": "city A"}, + { + "role": "tool", + "tool_call_id": "weather__astrbot_2", + "content": "city B", + }, + ] + } + + contents = provider._prepare_conversation(payloads) + function_response_names = [ + part.function_response.name + for content in contents + for part in content.parts or [] + if part.function_response + ] + + assert function_response_names == ["weather", "weather"] + + @pytest.mark.asyncio async def test_gemini_get_models_retries_transient_request_error(monkeypatch): monkeypatch.setattr(request_retry, "REQUEST_RETRY_WAIT_MIN_S", 0)