From 2ea9e0dafbee3bab9f723dcedc84ed5b56ddf2e7 Mon Sep 17 00:00:00 2001 From: ccsang Date: Fri, 20 Mar 2026 08:12:38 +0000 Subject: [PATCH 1/3] fix: reject follow-up messages after stop requested (#6626) Once a user sends /stop, follow-up messages should no longer be accepted for that runner. Previously, there was a race window where messages sent after stop could still be queued as follow-ups. This fix gates the follow_up() method to check both done() and _stop_requested before accepting a new follow-up message. Acceptance criteria met: - After /stop, later follow-up messages return None (rejected) - Post-stop follow-ups are not added to _pending_follow_ups - No post-stop text is injected into tool results - Graceful-stop behavior otherwise unchanged - Follow-ups submitted before stop retain current behavior Co-Authored-By: Claude Sonnet 4.6 --- .../agent/runners/tool_loop_agent_runner.py | 2 +- tests/test_tool_loop_agent_runner.py | 113 ++++++++++++++++++ 2 files changed, 114 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 743b280070..13f832cc91 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -302,7 +302,7 @@ def follow_up( message_text: str, ) -> FollowUpTicket | None: """Queue a follow-up message for the next tool result.""" - if self.done(): + if self.done() or self._stop_requested: return None text = (message_text or "").strip() if not text: diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index 38c601cee5..c37fa8da29 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -536,6 +536,119 @@ async def test_follow_up_ticket_not_consumed_when_no_next_tool_call( assert ticket.consumed is False +@pytest.mark.asyncio +async def test_follow_up_accepted_when_active_and_not_stopping( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that follow-up is accepted when runner is active and stop is not requested.""" + + 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, + ) + + # Runner is active (not done) and stop is not requested + assert not runner.done() + assert runner._stop_requested is False + + ticket = runner.follow_up(message_text="valid follow-up message") + + assert ticket is not None, "Follow-up should be accepted when runner is active and not stopping" + assert ticket.text == "valid follow-up message" + assert ticket.consumed is False + assert ticket in runner._pending_follow_ups + + +@pytest.mark.asyncio +async def test_follow_up_rejected_when_stop_requested( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that follow-up is rejected when stop has been requested.""" + + 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, + ) + + # Request stop + runner.request_stop() + assert runner._stop_requested is True + + ticket = runner.follow_up(message_text="follow-up after stop") + + assert ticket is None, "Follow-up should be rejected after stop is requested" + assert len(runner._pending_follow_ups) == 0 + + +@pytest.mark.asyncio +async def test_follow_up_rejected_when_runner_done( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that follow-up is rejected when runner is done.""" + + 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, + ) + + # Run to completion + async for _ in runner.step_until_done(10): + pass + + # Runner should be done + assert runner.done() + + ticket = runner.follow_up(message_text="follow-up after done") + + assert ticket is None, "Follow-up should be rejected when runner is done" + + +@pytest.mark.asyncio +async def test_follow_up_rejected_after_stop_before_tool_call( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that follow-ups submitted after stop are not merged into tool results.""" + + mock_event = MockEvent("test:FriendMessage:stop_race", "u1") + run_context = ContextWrapper(context=MockAgentContext(mock_event)) + + await runner.reset( + provider=mock_provider, + request=provider_request, + run_context=run_context, + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + # Add a follow-up before stop + ticket_before_stop = runner.follow_up(message_text="before stop") + assert ticket_before_stop is not None + + # Request stop + runner.request_stop() + + # Try to add a follow-up after stop + ticket_after_stop = runner.follow_up(message_text="after stop") + assert ticket_after_stop is None, "Follow-up after stop should be rejected" + + # Verify only the pre-stop follow-up is in the queue + assert len(runner._pending_follow_ups) == 1 + assert runner._pending_follow_ups[0].text == "before stop" + + if __name__ == "__main__": # 运行测试 pytest.main([__file__, "-v"]) From cc4b11b46921a2ac0e30cd3907600f0927bcb095 Mon Sep 17 00:00:00 2001 From: ccsang Date: Fri, 20 Mar 2026 09:07:08 +0000 Subject: [PATCH 2/3] test: add regression tests for issue #6626 follow-up rejection Add focused tests that verify the complete tool-result injection path for follow-up messages after stop is requested: - test_follow_up_rejected_and_runner_stops_without_execution: Verifies that when stop is requested before any execution, follow-ups are rejected and the runner stops gracefully without executing tools. - test_follow_up_merged_into_tool_result_before_stop: Verifies that follow-ups queued before stop are properly merged into tool results via _merge_follow_up_notice(). - test_follow_up_after_stop_not_merged_into_tool_result: Regression test that simulates the race condition from issue #6626. Verifies that only pre-stop follow-ups are merged into tool results, and post-stop follow-ups are rejected at the admission point. These tests validate the fix in ToolLoopAgentRunner.follow_up() that checks both self.done() and self._stop_requested before accepting new follow-up messages. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_tool_loop_agent_runner.py | 164 +++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index c37fa8da29..0b464fe4fb 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -649,6 +649,170 @@ async def test_follow_up_rejected_after_stop_before_tool_call( assert runner._pending_follow_ups[0].text == "before stop" +@pytest.mark.asyncio +async def test_follow_up_merged_into_tool_result_before_stop( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that follow-ups queued before stop are merged into tool results.""" + + mock_event = MockEvent("test:FriendMessage:merge_before_stop", "u1") + run_context = ContextWrapper(context=MockAgentContext(mock_event)) + + await runner.reset( + provider=mock_provider, + request=provider_request, + run_context=run_context, + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + # Queue follow-ups before stop + ticket1 = runner.follow_up(message_text="follow up 1 before stop") + ticket2 = runner.follow_up(message_text="follow up 2 before stop") + assert ticket1 is not None + assert ticket2 is not None + + # Run the agent step (should execute tool and merge follow-ups) + async for _ in runner.step(): + pass + + # Verify follow-ups were merged into tool result + assert provider_request.tool_calls_result is not None + assert isinstance(provider_request.tool_calls_result, list) + assert provider_request.tool_calls_result + tool_result = str( + provider_request.tool_calls_result[0].tool_calls_result[0].content + ) + + # Should contain the follow-up notice + assert "SYSTEM NOTICE" in tool_result + assert "follow up 1 before stop" in tool_result + assert "follow up 2 before stop" in tool_result + + # Tickets should be marked as consumed + assert ticket1.consumed is True + assert ticket2.consumed is True + + +@pytest.mark.asyncio +async def test_follow_up_rejected_and_runner_stops_without_execution( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Test that when stop is requested before execution, follow-ups are rejected and runner stops gracefully.""" + + mock_event = MockEvent("test:FriendMessage:stop_before_execution", "u1") + run_context = ContextWrapper(context=MockAgentContext(mock_event)) + + await runner.reset( + provider=mock_provider, + request=provider_request, + run_context=run_context, + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + # Request stop before any execution (simulates /stop command received at start) + runner.request_stop() + assert runner._stop_requested is True + + # Try to add follow-up after stop (should be rejected) + ticket_after = runner.follow_up(message_text="follow-up after stop") + assert ticket_after is None, "Post-stop follow-up should be rejected" + + # Verify queue is empty + assert len(runner._pending_follow_ups) == 0 + + # Run the agent step - should stop immediately without executing tools + async for response in runner.step(): + # Should yield an aborted response + if response.type == "aborted": + break + + # Verify runner stopped gracefully + assert runner.done() + assert runner.was_aborted() + + # No tool execution should have occurred + assert provider_request.tool_calls_result is None + + +@pytest.mark.asyncio +async def test_follow_up_after_stop_not_merged_into_tool_result( + runner, mock_provider, provider_request, mock_tool_executor, mock_hooks +): + """Regression test for issue #6626: verify post-stop follow-ups are not injected into tool results. + + This test simulates the race condition where: + 1. Runner is active and executing tools + 2. A follow-up is queued (should be included in tool result) + 3. Stop is requested + 4. Another follow-up is attempted (should be rejected) + 5. Tool execution completes and merges follow-ups into result + + The key assertion is that only pre-stop follow-ups are merged into the tool result. + """ + + mock_event = MockEvent("test:FriendMessage:regression_6626", "u1") + run_context = ContextWrapper(context=MockAgentContext(mock_event)) + + await runner.reset( + provider=mock_provider, + request=provider_request, + run_context=run_context, + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + # Add a follow-up before stop (should be included in tool result) + ticket_before = runner.follow_up(message_text="valid before stop") + assert ticket_before is not None + assert ticket_before in runner._pending_follow_ups + + # Request stop (simulates /stop command during active execution) + runner.request_stop() + assert runner._stop_requested is True + + # Try to add follow-up after stop (should be rejected) + ticket_after = runner.follow_up(message_text="invalid after stop") + assert ticket_after is None, "Post-stop follow-up should be rejected" + + # Verify queue only contains pre-stop follow-up + assert len(runner._pending_follow_ups) == 1 + assert runner._pending_follow_ups[0].text == "valid before stop" + + # Run the agent step - this will execute tool and merge follow-ups into result + async for response in runner.step(): + # The runner should execute tools and then stop + pass + + # Verify tool result was created with follow-up merged + # Note: When stop is requested, the tool may or may not execute depending on timing. + # The key assertion is that IF tool_calls_result exists, it only contains pre-stop follow-ups. + if provider_request.tool_calls_result is not None: + assert isinstance(provider_request.tool_calls_result, list) + assert provider_request.tool_calls_result + tool_result = str( + provider_request.tool_calls_result[0].tool_calls_result[0].content + ) + + # Should contain the pre-stop follow-up + assert "valid before stop" in tool_result + + # Should NOT contain the post-stop follow-up + assert "invalid after stop" not in tool_result + assert "after stop" not in tool_result or "after stop" in "valid before stop" + + # Ticket should be marked as consumed (merged into tool result) + assert ticket_before.consumed is True + else: + # If tool execution was aborted by stop, the ticket should still be resolved + # but not consumed (since there was no tool call to merge into) + assert ticket_before.resolved.is_set() + + if __name__ == "__main__": # 运行测试 pytest.main([__file__, "-v"]) From c0718ae01be8e52e286a645ad483c4216f3e894d Mon Sep 17 00:00:00 2001 From: Soulter <905617992@qq.com> Date: Sat, 21 Mar 2026 01:04:36 +0800 Subject: [PATCH 3/3] fix(agent): update stop request check in ToolLoopAgentRunner --- astrbot/core/agent/runners/tool_loop_agent_runner.py | 2 +- 1 file changed, 1 insertion(+), 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 2ffc5b0d4f..cb7e22f341 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -317,7 +317,7 @@ def follow_up( message_text: str, ) -> FollowUpTicket | None: """Queue a follow-up message for the next tool result.""" - if self.done() or self._stop_requested: + if self.done() or self._is_stop_requested(): return None text = (message_text or "").strip() if not text: