From 95449178e8454f241e00119208ce6f33c3e7ffc5 Mon Sep 17 00:00:00 2001 From: xiaoyuyu6420 <93528429+xiaoyuyu6420@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:59:36 +0800 Subject: [PATCH 1/4] fix(cron): deliver proactive agent final text to the bound session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cron and background-task wakeups consumed runner.step_until_done() with a discarded iterator, so the final assistant text was only folded into the persisted history summary and never sent to the delivery session. At forced wrap-up (max steps) all tools are removed, so the model cannot use send_message_to_user either — the user only ever saw intermediate messages (#9980). Deliver the final assistant text through the cron event after history persistence on both wake paths, skipping when the model already sent this exact text to the current session via send_message_to_user during the same run (recorded by the tool as SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY). A failed send is logged but does not fail the job. Signed-off-by: xiaoyuyu6420 --- astrbot/core/astr_agent_tool_exec.py | 21 +++ astrbot/core/cron/manager.py | 25 ++- astrbot/core/tools/message_tools.py | 13 +- tests/unit/test_cron_manager.py | 250 +++++++++++++++++++++++++++ 4 files changed, 306 insertions(+), 3 deletions(-) diff --git a/astrbot/core/astr_agent_tool_exec.py b/astrbot/core/astr_agent_tool_exec.py index 408ee71268..357011a0fd 100644 --- a/astrbot/core/astr_agent_tool_exec.py +++ b/astrbot/core/astr_agent_tool_exec.py @@ -532,6 +532,9 @@ async def _wake_main_agent_for_background_result( _get_session_conv, build_main_agent, ) + from astrbot.core.tools.message_tools import ( + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, + ) event = run_context.context.event ctx = run_context.context.context @@ -639,6 +642,24 @@ async def _wake_main_agent_for_background_result( logger.warning("background task agent got no response") return + final_text = (llm_resp.completion_text or "").strip() + if llm_resp.role == "assistant" and final_text: + # Same delivery gap as the cron path (#9980): the final text only + # lands in persisted history unless it is explicitly sent. Skip + # when the model already delivered this exact text via + # send_message_to_user earlier in the same run. + already_sent = cron_event.get_extra( + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [] + ) + if final_text not in already_sent: + try: + await cron_event.send(MessageChain().message(final_text)) + except Exception as e: # noqa: BLE001 + logger.warning( + f"Failed to deliver background task final response: {e}", + exc_info=True, + ) + @classmethod async def _execute_local( cls, diff --git a/astrbot/core/cron/manager.py b/astrbot/core/cron/manager.py index 8515224b5d..701734811b 100644 --- a/astrbot/core/cron/manager.py +++ b/astrbot/core/cron/manager.py @@ -17,6 +17,7 @@ from astrbot.core.cron.events import CronMessageEvent from astrbot.core.db import BaseDatabase from astrbot.core.db.po import CronJob +from astrbot.core.message.message_event_result import MessageChain from astrbot.core.platform.message_session import MessageSession from astrbot.core.platform.message_type import MessageType from astrbot.core.provider.entites import ProviderRequest @@ -412,7 +413,10 @@ async def _woke_main_agent( from astrbot.core.astr_main_agent_resources import ( PROACTIVE_AGENT_CRON_WOKE_SYSTEM_PROMPT, ) - from astrbot.core.tools.message_tools import SendMessageToUserTool + from astrbot.core.tools.message_tools import ( + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, + SendMessageToUserTool, + ) try: session = ( @@ -539,5 +543,24 @@ async def _woke_main_agent( logger.warning("Cron job agent got no response") return + final_text = (llm_resp.completion_text or "").strip() + if llm_resp.role == "assistant" and final_text and delivery_session_str: + # The runner's final text is only folded into the persisted history; + # without an explicit send the bound session never receives it, + # which is exactly the "only intermediate messages" symptom of + # #9980. Skip when the model already delivered this exact text via + # send_message_to_user earlier in the same run. + already_sent = cron_event.get_extra( + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [] + ) + if final_text not in already_sent: + try: + await cron_event.send(MessageChain().message(final_text)) + except Exception as e: # noqa: BLE001 + logger.warning( + f"Failed to deliver cron agent final response: {e}", + exc_info=True, + ) + __all__ = ["CronJobManager"] diff --git a/astrbot/core/tools/message_tools.py b/astrbot/core/tools/message_tools.py index ca6f21e9b5..11304043b0 100644 --- a/astrbot/core/tools/message_tools.py +++ b/astrbot/core/tools/message_tools.py @@ -52,6 +52,15 @@ def _is_path_within(path: Path, roots: tuple[Path, ...]) -> bool: return any(path == root or path.is_relative_to(root) for root in roots) +# Extra key on the event recording plain texts already delivered to the current +# session via send_message_to_user during this agent run. Proactive callers +# (cron jobs, background task wakeups) read it to avoid re-sending the final +# assistant text the model already delivered through the tool. +SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY = ( + "_send_message_to_user_current_session_plain_texts" +) + + def _is_restricted_local_env(context: ContextWrapper[AstrAgentContext]) -> bool: if not is_local_runtime(context): return False @@ -349,14 +358,14 @@ async def call( sent_plain_text = message_chain.get_plain_text().strip() if sent_plain_text: sent_plain_texts = context.context.event.get_extra( - "_send_message_to_user_current_session_plain_texts", + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [], ) if not isinstance(sent_plain_texts, list): sent_plain_texts = [] sent_plain_texts.append(sent_plain_text) context.context.event.set_extra( - "_send_message_to_user_current_session_plain_texts", + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, sent_plain_texts, ) return f"Message sent to session {target_session}" diff --git a/tests/unit/test_cron_manager.py b/tests/unit/test_cron_manager.py index 0dcb480d77..ebb627da7d 100644 --- a/tests/unit/test_cron_manager.py +++ b/tests/unit/test_cron_manager.py @@ -15,6 +15,10 @@ _normalize_crontab_day_of_week, ) from astrbot.core.db.po import CronJob +from astrbot.core.provider.entities import LLMResponse +from astrbot.core.tools.message_tools import ( + SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, +) @pytest.fixture @@ -816,6 +820,252 @@ async def test_run_basic_job_no_handler(self, cron_manager, sample_cron_job): await cron_manager._run_basic_job(sample_cron_job) +class TestWokeMainAgentFinalDelivery: + """The cron agent's final text must reach the bound session (#9980).""" + + @pytest.mark.asyncio + async def test_delivers_final_text_to_delivery_session(self, cron_manager): + """A finished cron agent's final assistant text is sent to the session.""" + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse(role="assistant", completion_text="Done: 42") + + event_box = {} + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event_box["event"] = event + event.send = AsyncMock() + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str="test:FriendMessage:user123", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str="test:FriendMessage:user123", + ) + + event_box["event"].send.assert_awaited_once() + chain = event_box["event"].send.await_args.args[0] + assert chain.get_plain_text() == "Done: 42" + + @pytest.mark.asyncio + async def test_skips_delivery_when_tool_already_sent_same_text(self, cron_manager): + """No duplicate send when the model delivered the text via the tool.""" + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse(role="assistant", completion_text="Done: 42") + + event_box = {} + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event_box["event"] = event + event.send = AsyncMock() + # Simulate send_message_to_user having delivered the same text. + event.set_extra(SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, ["Done: 42"]) + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str="test:FriendMessage:user123", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str="test:FriendMessage:user123", + ) + + event_box["event"].send.assert_not_awaited() + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("llm_role", "completion_text", "delivery_session_str"), + [ + pytest.param("assistant", "Done: 42", "", id="no_delivery_session"), + pytest.param( + "assistant", "", "test:FriendMessage:user123", id="empty_text" + ), + pytest.param( + "tool", "Done: 42", "test:FriendMessage:user123", id="non_assistant" + ), + ], + ) + async def test_skips_delivery_for_non_deliverable_responses( + self, cron_manager, llm_role, completion_text, delivery_session_str + ): + """Nothing is sent when there is no target, no text, or no assistant reply.""" + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse(role=llm_role, completion_text=completion_text) + + event_box = {} + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event_box["event"] = event + event.send = AsyncMock() + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str=delivery_session_str or "cron:OtherMessage:job-1", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str=delivery_session_str, + ) + + event_box["event"].send.assert_not_awaited() + + @pytest.mark.asyncio + async def test_delivery_failure_does_not_fail_the_job(self, cron_manager): + """A failed send is logged but must not mark the job as failed.""" + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse(role="assistant", completion_text="Done: 42") + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event.send = AsyncMock(side_effect=RuntimeError("platform offline")) + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + # Must not raise. + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str="test:FriendMessage:user123", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str="test:FriendMessage:user123", + ) + + class TestGetNextRunTime: """Tests for _get_next_run_time method.""" From a11ce12f1d039a06d150a039f74f42c38483a8e5 Mon Sep 17 00:00:00 2001 From: xiaoyuyu6420 <93528429+xiaoyuyu6420@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:40:02 +0800 Subject: [PATCH 2/4] fix(cron): deliver final text to delivery_session_str explicitly Address Sourcery bug_risk: delivery went through cron_event (built from session_str) while the gate checked delivery_session_str. When a caller passes differing values the text would land on the wrong session. Send via Context.send_message(delivery_session_str, ...) so the gate and the target are the same variable; no-platform and failure outcomes are logged without failing the job. Signed-off-by: xiaoyuyu6420 --- astrbot/core/cron/manager.py | 16 +++++-- tests/unit/test_cron_manager.py | 76 +++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/astrbot/core/cron/manager.py b/astrbot/core/cron/manager.py index 701734811b..8edd525e04 100644 --- a/astrbot/core/cron/manager.py +++ b/astrbot/core/cron/manager.py @@ -548,14 +548,24 @@ async def _woke_main_agent( # The runner's final text is only folded into the persisted history; # without an explicit send the bound session never receives it, # which is exactly the "only intermediate messages" symptom of - # #9980. Skip when the model already delivered this exact text via - # send_message_to_user earlier in the same run. + # #9980. Send to delivery_session_str (not the event session, which + # falls back to a synthetic cron session) and skip when the model + # already delivered this exact text via send_message_to_user + # earlier in the same run. already_sent = cron_event.get_extra( SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [] ) if final_text not in already_sent: try: - await cron_event.send(MessageChain().message(final_text)) + sent = await self.ctx.send_message( + delivery_session_str, + MessageChain().message(final_text), + ) + if not sent: + logger.warning( + "Failed to deliver cron agent final response: " + f"no platform found for session {delivery_session_str}" + ) except Exception as e: # noqa: BLE001 logger.warning( f"Failed to deliver cron agent final response: {e}", diff --git a/tests/unit/test_cron_manager.py b/tests/unit/test_cron_manager.py index ebb627da7d..35934bd7a1 100644 --- a/tests/unit/test_cron_manager.py +++ b/tests/unit/test_cron_manager.py @@ -835,6 +835,7 @@ async def test_delivers_final_text_to_delivery_session(self, cron_manager): "config": {"misc": {}, "compression": {}}, }, } + ctx.send_message = AsyncMock(return_value=True) cron_manager.ctx = ctx conv = MagicMock() @@ -878,9 +879,11 @@ async def fake_build_main_agent(*, event, plugin_context, config, req): delivery_session_str="test:FriendMessage:user123", ) - event_box["event"].send.assert_awaited_once() - chain = event_box["event"].send.await_args.args[0] + ctx.send_message.assert_awaited_once() + target_session, chain = ctx.send_message.await_args.args + assert str(target_session) == "test:FriendMessage:user123" assert chain.get_plain_text() == "Done: 42" + event_box["event"].send.assert_not_awaited() @pytest.mark.asyncio async def test_skips_delivery_when_tool_already_sent_same_text(self, cron_manager): @@ -940,6 +943,7 @@ async def fake_build_main_agent(*, event, plugin_context, config, req): ) event_box["event"].send.assert_not_awaited() + ctx.send_message.assert_not_called() @pytest.mark.asyncio @pytest.mark.parametrize( @@ -1010,6 +1014,70 @@ async def fake_build_main_agent(*, event, plugin_context, config, req): delivery_session_str=delivery_session_str, ) + ctx.send_message.assert_not_called() + + @pytest.mark.asyncio + async def test_delivery_targets_delivery_session_not_event_session( + self, cron_manager + ): + """The text goes to delivery_session_str even when it differs from the + event's session (which otherwise falls back to a synthetic one).""" + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + ctx.send_message = AsyncMock(return_value=True) + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse(role="assistant", completion_text="Done: 42") + + event_box = {} + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event_box["event"] = event + event.send = AsyncMock() + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str="cron:OtherMessage:job-1", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str="test:FriendMessage:user123", + ) + + ctx.send_message.assert_awaited_once() + target_session, _ = ctx.send_message.await_args.args + assert str(target_session) == "test:FriendMessage:user123" event_box["event"].send.assert_not_awaited() @pytest.mark.asyncio @@ -1040,9 +1108,11 @@ def get_final_llm_resp(self): return LLMResponse(role="assistant", completion_text="Done: 42") async def fake_build_main_agent(*, event, plugin_context, config, req): - event.send = AsyncMock(side_effect=RuntimeError("platform offline")) + event.send = AsyncMock() return MagicMock(agent_runner=FakeRunner()) + ctx.send_message = AsyncMock(side_effect=RuntimeError("platform offline")) + with ( patch( "astrbot.core.astr_main_agent._get_session_conv", From 0b030f9d2252b420039ba00b5415755c4b91e021 Mon Sep 17 00:00:00 2001 From: xiaoyuyu6420 <93528429+xiaoyuyu6420@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:16:55 +0800 Subject: [PATCH 3/4] fix(cron): restrict final-text delivery to the forced wrap-up path Review feedback on #9980: blanket delivery of the final assistant text also fired on normally completed runs, where the model still has send_message_to_user available and may intentionally stay silent (e.g. conditional notify-only cron jobs). DONE alone does not imply the user should be notified. ToolLoopAgentRunner now exposes reached_max_steps, set only in the forced final-response branch of step_until_done where all tools were removed and the model had no channel to deliver its summary. Cron and background-task wakeups deliver the final text only in that case; normal completion keeps the current silent behavior. Signed-off-by: xiaoyuyu6420 --- .../agent/runners/tool_loop_agent_runner.py | 12 +++ astrbot/core/astr_agent_tool_exec.py | 16 ++-- astrbot/core/cron/manager.py | 21 ++++-- tests/unit/test_cron_manager.py | 73 +++++++++++++++++++ 4 files changed, 109 insertions(+), 13 deletions(-) diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 3c4cab9046..24a078be21 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -233,6 +233,7 @@ async def reset( ) -> None: self.req = request self.streaming = streaming + self._reached_max_steps = False self.enforce_max_turns = enforce_max_turns self.llm_compress_instruction = llm_compress_instruction self.llm_compress_keep_recent_ratio = llm_compress_keep_recent_ratio @@ -1068,6 +1069,16 @@ async def step(self): self.req.append_tool_calls_result(tool_calls_result) + @property + def reached_max_steps(self) -> bool: + """Whether this run ended via the forced final response at max steps. + + Only meaningful after ``step_until_done`` has run; a forced wrap-up + removes all tools, so any final text the model produced there could + not be delivered through ``send_message_to_user``. + """ + return self._reached_max_steps + async def step_until_done( self, max_step: int ) -> T.AsyncGenerator[AgentResponse, None]: @@ -1083,6 +1094,7 @@ async def step_until_done( logger.warning( f"Agent reached max steps ({max_step}), forcing a final response." ) + self._reached_max_steps = True # 拔掉所有工具 if self.req: self.req.func_tool = None diff --git a/astrbot/core/astr_agent_tool_exec.py b/astrbot/core/astr_agent_tool_exec.py index 357011a0fd..a85b3a338d 100644 --- a/astrbot/core/astr_agent_tool_exec.py +++ b/astrbot/core/astr_agent_tool_exec.py @@ -643,11 +643,17 @@ async def _wake_main_agent_for_background_result( return final_text = (llm_resp.completion_text or "").strip() - if llm_resp.role == "assistant" and final_text: - # Same delivery gap as the cron path (#9980): the final text only - # lands in persisted history unless it is explicitly sent. Skip - # when the model already delivered this exact text via - # send_message_to_user earlier in the same run. + if ( + getattr(runner, "reached_max_steps", False) + and llm_resp.role == "assistant" + and final_text + ): + # Same delivery gap as the cron path (#9980), and same narrowing: + # only the forced wrap-up at max steps removed every tool, so only + # there the final text could not have been sent via + # send_message_to_user. On normal completion the model may + # intentionally stay silent. Skip when this exact text was already + # delivered earlier in the same run. already_sent = cron_event.get_extra( SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [] ) diff --git a/astrbot/core/cron/manager.py b/astrbot/core/cron/manager.py index 8edd525e04..4942a96cb2 100644 --- a/astrbot/core/cron/manager.py +++ b/astrbot/core/cron/manager.py @@ -544,14 +544,19 @@ async def _woke_main_agent( return final_text = (llm_resp.completion_text or "").strip() - if llm_resp.role == "assistant" and final_text and delivery_session_str: - # The runner's final text is only folded into the persisted history; - # without an explicit send the bound session never receives it, - # which is exactly the "only intermediate messages" symptom of - # #9980. Send to delivery_session_str (not the event session, which - # falls back to a synthetic cron session) and skip when the model - # already delivered this exact text via send_message_to_user - # earlier in the same run. + if ( + getattr(runner, "reached_max_steps", False) + and llm_resp.role == "assistant" + and final_text + and delivery_session_str + ): + # Deliver ONLY on the forced wrap-up at max steps: there all tools + # were removed, so the model had no channel to reach the user even + # if it wanted to (#9980). On normal completion the model still has + # send_message_to_user available and may intentionally stay silent + # (e.g. conditional notify-only jobs), so the framework must not + # decide for it. Skip when the model already delivered this exact + # text earlier in the same run. already_sent = cron_event.get_extra( SENT_TO_CURRENT_SESSION_PLAIN_TEXTS_EXTRA_KEY, [] ) diff --git a/tests/unit/test_cron_manager.py b/tests/unit/test_cron_manager.py index 35934bd7a1..186adfa44c 100644 --- a/tests/unit/test_cron_manager.py +++ b/tests/unit/test_cron_manager.py @@ -843,6 +843,7 @@ async def test_delivers_final_text_to_delivery_session(self, cron_manager): class FakeRunner: state = AgentState.DONE + reached_max_steps = True async def step_until_done(self, max_step): return @@ -904,6 +905,7 @@ async def test_skips_delivery_when_tool_already_sent_same_text(self, cron_manage class FakeRunner: state = AgentState.DONE + reached_max_steps = True async def step_until_done(self, max_step): return @@ -978,6 +980,7 @@ async def test_skips_delivery_for_non_deliverable_responses( class FakeRunner: state = AgentState.DONE + reached_max_steps = True async def step_until_done(self, max_step): return @@ -1039,6 +1042,7 @@ async def test_delivery_targets_delivery_session_not_event_session( class FakeRunner: state = AgentState.DONE + reached_max_steps = True async def step_until_done(self, max_step): return @@ -1080,6 +1084,74 @@ async def fake_build_main_agent(*, event, plugin_context, config, req): assert str(target_session) == "test:FriendMessage:user123" event_box["event"].send.assert_not_awaited() + @pytest.mark.asyncio + async def test_normal_completion_stays_silent(self, cron_manager): + """On normal completion the framework must not deliver the final text. + + The model still has send_message_to_user available and may intend to + stay silent (e.g. a conditional notify-only cron job), so DONE alone + does not imply the user should be notified. + """ + ctx = MagicMock() + ctx.get_config.return_value = { + "admins_id": [], + "provider_settings": {}, + "agent_runner": { + "runner_type": "local", + "config": {"misc": {}, "compression": {}}, + }, + } + ctx.send_message = AsyncMock(return_value=True) + cron_manager.ctx = ctx + + conv = MagicMock() + conv.history = "[]" + + class FakeRunner: + state = AgentState.DONE + reached_max_steps = False + + async def step_until_done(self, max_step): + return + yield # pragma: no cover + + def get_final_llm_resp(self): + return LLMResponse( + role="assistant", + completion_text="Not reached yet, no notification needed", + ) + + event_box = {} + + async def fake_build_main_agent(*, event, plugin_context, config, req): + event_box["event"] = event + event.send = AsyncMock() + return MagicMock(agent_runner=FakeRunner()) + + with ( + patch( + "astrbot.core.astr_main_agent._get_session_conv", + AsyncMock(return_value=conv), + ), + patch( + "astrbot.core.astr_main_agent.build_main_agent", + side_effect=fake_build_main_agent, + ), + patch( + "astrbot.core.cron.manager.persist_agent_history", + AsyncMock(), + ), + ): + await cron_manager._woke_main_agent( + message="run scheduled task", + session_str="test:FriendMessage:user123", + extras={"cron_job": {"id": "job-1"}, "cron_payload": {}}, + delivery_session_str="test:FriendMessage:user123", + ) + + ctx.send_message.assert_not_called() + event_box["event"].send.assert_not_awaited() + @pytest.mark.asyncio async def test_delivery_failure_does_not_fail_the_job(self, cron_manager): """A failed send is logged but must not mark the job as failed.""" @@ -1099,6 +1171,7 @@ async def test_delivery_failure_does_not_fail_the_job(self, cron_manager): class FakeRunner: state = AgentState.DONE + reached_max_steps = True async def step_until_done(self, max_step): return From 37a68dccfa637d4b124d32fe3d75d8abf7488bb6 Mon Sep 17 00:00:00 2001 From: xiaoyuyu6420 <93528429+xiaoyuyu6420@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:04:10 +0800 Subject: [PATCH 4/4] fix(agent): make reached_max_steps safe to read before reset reached_max_steps was only initialized in reset(), so reading the property on a fresh runner raised AttributeError instead of returning the documented default. Add a class-level False default and tests covering both the forced wrap-up and normal-completion paths against a real runner. Signed-off-by: xiaoyuyu6420 --- .../agent/runners/tool_loop_agent_runner.py | 2 + tests/test_tool_loop_agent_runner.py | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 24a078be21..433098abaf 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -127,6 +127,8 @@ class ToolLoopAgentRunner(BaseAgentRunner[TContext]): "Stop calling tools, and based on the information you have gathered, " "summarize your task and findings, and reply to the user directly." ) + # Class-level default so the read-only property is safe before reset(). + _reached_max_steps: bool = False SKILLS_LIKE_REQUERY_INSTRUCTION_TEMPLATE = ( "You have decided to call tool(s): {tool_names}. Now call the tool(s) " "with required arguments using the tool schema, and follow the existing " diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index b21ed40d82..a9b51463f8 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -665,6 +665,52 @@ async def test_normal_completion_without_max_step( assert runner.req.func_tool is not None, "正常完成时工具不应该被禁用" +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("max_calls_before_normal_response", "max_steps", "expected_reached"), + [ + pytest.param(100, 3, True, id="forced_wrap_up_sets_flag"), + pytest.param(2, 10, False, id="normal_completion_keeps_flag_false"), + ], +) +async def test_reached_max_steps_flag( + runner, + mock_provider, + provider_request, + mock_tool_executor, + mock_hooks, + max_calls_before_normal_response, + max_steps, + expected_reached, +): + """reached_max_steps 只在强制收尾分支置位，正常完成保持 False。 + + 下游（cron / 后台唤醒）依赖该标记决定是否补投最终文本：正常结束时 + 模型仍有 send_message_to_user 可用，可能有意保持静默。 + """ + assert runner.reached_max_steps is False + + mock_provider.should_call_tools = True + mock_provider.max_calls_before_normal_response = max_calls_before_normal_response + + await runner.reset( + provider=mock_provider, + request=provider_request, + run_context=ContextWrapper(context=None), + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + async for _ in runner.step_until_done(max_steps): + pass + + assert runner.done() + assert runner.reached_max_steps is expected_reached + # 标记与"工具被拔掉"这一强制收尾特征严格一致 + assert (runner.req.func_tool is None) is expected_reached + + @pytest.mark.asyncio @pytest.mark.parametrize("streaming", [False, True]) async def test_stats_separate_latest_context_from_cumulative_usage(