feat(agent): mid-turn message injection for responsive follow-ups (#2985)
* feat(agent): add mid-turn message injection for responsive follow-ups
Allow user messages sent during an active agent turn to be injected
into the running LLM context instead of being queued behind a
per-session lock. Inspired by Claude Code's mid-turn queue drain
mechanism (query.ts:1547-1643).
Key design decisions:
- Messages are injected as natural user messages between iterations,
no tool cancellation or special system prompt needed
- Two drain checkpoints: after tool execution and after final LLM
response ("last-mile" to prevent dropping late arrivals)
- Bounded by MAX_INJECTION_CYCLES (5) to prevent consuming the
iteration budget on rapid follow-ups
- had_injections flag bypasses _sent_in_turn suppression so follow-up
responses are always delivered
Closes #1609
* fix(agent): harden mid-turn injection with streaming fix, bounded queue, and message safety
- Fix streaming protocol violation: Checkpoint 2 now checks for injections
BEFORE calling on_stream_end, passing resuming=True when injections found
so streaming channels (Feishu) don't prematurely finalize the card
- Bound pending queue to maxsize=20 with QueueFull handling
- Add warning log when injection batch exceeds _MAX_INJECTIONS_PER_TURN
- Re-publish leftover queue messages to bus in _dispatch finally block to
prevent silent message loss on early exit (max_iterations, tool_error, cancel)
- Fix PEP 8 blank line before dataclass and logger.info indentation
- Add 12 new tests covering drain, checkpoints, cycle cap, queue routing,
cleanup, and leftover re-publish
This commit is contained in:
@@ -307,7 +307,7 @@ async def test_agent_loop_extra_hook_receives_calls(tmp_path):
|
||||
)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
|
||||
content, tools_used, messages, _ = await loop._run_agent_loop(
|
||||
content, tools_used, messages, _, _ = await loop._run_agent_loop(
|
||||
[{"role": "user", "content": "hi"}]
|
||||
)
|
||||
|
||||
@@ -331,7 +331,7 @@ async def test_agent_loop_extra_hook_error_isolation(tmp_path):
|
||||
)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
|
||||
content, _, _, _ = await loop._run_agent_loop(
|
||||
content, _, _, _, _ = await loop._run_agent_loop(
|
||||
[{"role": "user", "content": "hi"}]
|
||||
)
|
||||
|
||||
@@ -373,7 +373,7 @@ async def test_agent_loop_no_hooks_backward_compat(tmp_path):
|
||||
loop.tools.execute = AsyncMock(return_value="ok")
|
||||
loop.max_iterations = 2
|
||||
|
||||
content, tools_used, _, _ = await loop._run_agent_loop([])
|
||||
content, tools_used, _, _, _ = await loop._run_agent_loop([])
|
||||
assert content == (
|
||||
"I reached the maximum number of tool call iterations (2) "
|
||||
"without completing the task. You can try breaking the task into smaller steps."
|
||||
|
||||
+412
-3
@@ -798,7 +798,7 @@ async def test_loop_max_iterations_message_stays_stable(tmp_path):
|
||||
loop.tools.execute = AsyncMock(return_value="ok")
|
||||
loop.max_iterations = 2
|
||||
|
||||
final_content, _, _, _ = await loop._run_agent_loop([])
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop([])
|
||||
|
||||
assert final_content == (
|
||||
"I reached the maximum number of tool call iterations (2) "
|
||||
@@ -825,7 +825,7 @@ async def test_loop_stream_filter_handles_think_only_prefix_without_crashing(tmp
|
||||
async def on_stream_end(*, resuming: bool = False) -> None:
|
||||
endings.append(resuming)
|
||||
|
||||
final_content, _, _, _ = await loop._run_agent_loop(
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop(
|
||||
[],
|
||||
on_stream=on_stream,
|
||||
on_stream_end=on_stream_end,
|
||||
@@ -849,7 +849,7 @@ async def test_loop_retries_think_only_final_response(tmp_path):
|
||||
|
||||
loop.provider.chat_with_retry = chat_with_retry
|
||||
|
||||
final_content, _, _, _ = await loop._run_agent_loop([])
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop([])
|
||||
|
||||
assert final_content == "Recovered answer"
|
||||
assert call_count["n"] == 2
|
||||
@@ -1607,3 +1607,412 @@ async def test_microcompact_skips_non_compactable_tools():
|
||||
|
||||
result = AgentRunner._microcompact(messages)
|
||||
assert result is messages # no compactable tools found
|
||||
|
||||
|
||||
# ── Mid-turn injection tests ──────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_returns_empty_when_no_callback():
|
||||
"""No injection_callback → empty list."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
|
||||
provider = MagicMock()
|
||||
runner = AgentRunner(provider)
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
spec = AgentRunSpec(
|
||||
initial_messages=[], tools=tools, model="m",
|
||||
max_iterations=1, max_tool_result_chars=1000,
|
||||
injection_callback=None,
|
||||
)
|
||||
result = await runner._drain_injections(spec)
|
||||
assert result == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_extracts_content_from_inbound_messages():
|
||||
"""Should extract .content from InboundMessage objects."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner, _MAX_INJECTIONS_PER_TURN
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
runner = AgentRunner(provider)
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
msgs = [
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content="hello"),
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content="world"),
|
||||
]
|
||||
|
||||
async def cb():
|
||||
return msgs
|
||||
|
||||
spec = AgentRunSpec(
|
||||
initial_messages=[], tools=tools, model="m",
|
||||
max_iterations=1, max_tool_result_chars=1000,
|
||||
injection_callback=cb,
|
||||
)
|
||||
result = await runner._drain_injections(spec)
|
||||
assert result == ["hello", "world"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_caps_at_max_and_logs_warning():
|
||||
"""When more than _MAX_INJECTIONS_PER_TURN items, only the last N are kept."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner, _MAX_INJECTIONS_PER_TURN
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
runner = AgentRunner(provider)
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
msgs = [
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content=f"msg{i}")
|
||||
for i in range(_MAX_INJECTIONS_PER_TURN + 3)
|
||||
]
|
||||
|
||||
async def cb():
|
||||
return msgs
|
||||
|
||||
spec = AgentRunSpec(
|
||||
initial_messages=[], tools=tools, model="m",
|
||||
max_iterations=1, max_tool_result_chars=1000,
|
||||
injection_callback=cb,
|
||||
)
|
||||
result = await runner._drain_injections(spec)
|
||||
assert len(result) == _MAX_INJECTIONS_PER_TURN
|
||||
# Should keep the LAST _MAX_INJECTIONS_PER_TURN items
|
||||
assert result[0] == "msg3"
|
||||
assert result[-1] == f"msg{_MAX_INJECTIONS_PER_TURN + 2}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_skips_empty_content():
|
||||
"""Messages with blank content should be filtered out."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
runner = AgentRunner(provider)
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
msgs = [
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content=""),
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content=" "),
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content="valid"),
|
||||
]
|
||||
|
||||
async def cb():
|
||||
return msgs
|
||||
|
||||
spec = AgentRunSpec(
|
||||
initial_messages=[], tools=tools, model="m",
|
||||
max_iterations=1, max_tool_result_chars=1000,
|
||||
injection_callback=cb,
|
||||
)
|
||||
result = await runner._drain_injections(spec)
|
||||
assert result == ["valid"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_handles_callback_exception():
|
||||
"""If the callback raises, return empty list (error is logged)."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
|
||||
provider = MagicMock()
|
||||
runner = AgentRunner(provider)
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
async def cb():
|
||||
raise RuntimeError("boom")
|
||||
|
||||
spec = AgentRunSpec(
|
||||
initial_messages=[], tools=tools, model="m",
|
||||
max_iterations=1, max_tool_result_chars=1000,
|
||||
injection_callback=cb,
|
||||
)
|
||||
result = await runner._drain_injections(spec)
|
||||
assert result == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_checkpoint1_injects_after_tool_execution():
|
||||
"""Follow-up messages are injected after tool execution, before next LLM call."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
call_count = {"n": 0}
|
||||
captured_messages = []
|
||||
|
||||
async def chat_with_retry(*, messages, **kwargs):
|
||||
call_count["n"] += 1
|
||||
captured_messages.append(list(messages))
|
||||
if call_count["n"] == 1:
|
||||
return LLMResponse(
|
||||
content="using tool",
|
||||
tool_calls=[ToolCallRequest(id="c1", name="read_file", arguments={"path": "x"})],
|
||||
usage={},
|
||||
)
|
||||
return LLMResponse(content="final answer", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_with_retry = chat_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
tools.execute = AsyncMock(return_value="file content")
|
||||
|
||||
injection_queue = asyncio.Queue()
|
||||
|
||||
async def inject_cb():
|
||||
items = []
|
||||
while not injection_queue.empty():
|
||||
items.append(await injection_queue.get())
|
||||
return items
|
||||
|
||||
# Put a follow-up message in the queue before the run starts
|
||||
await injection_queue.put(
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content="follow-up question")
|
||||
)
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "hello"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=5,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
injection_callback=inject_cb,
|
||||
))
|
||||
|
||||
assert result.had_injections is True
|
||||
assert result.final_content == "final answer"
|
||||
# The second call should have the injected user message
|
||||
assert call_count["n"] == 2
|
||||
last_messages = captured_messages[-1]
|
||||
injected = [m for m in last_messages if m.get("role") == "user" and m.get("content") == "follow-up question"]
|
||||
assert len(injected) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_checkpoint2_injects_after_final_response_with_resuming_stream():
|
||||
"""After final response, if injections exist, stream_end should get resuming=True."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
call_count = {"n": 0}
|
||||
stream_end_calls = []
|
||||
|
||||
class TrackingHook(AgentHook):
|
||||
def wants_streaming(self) -> bool:
|
||||
return True
|
||||
|
||||
async def on_stream_end(self, context: AgentHookContext, *, resuming: bool) -> None:
|
||||
stream_end_calls.append(resuming)
|
||||
|
||||
def finalize_content(self, context: AgentHookContext, content: str | None) -> str | None:
|
||||
return content
|
||||
|
||||
async def chat_stream_with_retry(*, messages, on_content_delta=None, **kwargs):
|
||||
call_count["n"] += 1
|
||||
if call_count["n"] == 1:
|
||||
return LLMResponse(content="first answer", tool_calls=[], usage={})
|
||||
return LLMResponse(content="second answer", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
injection_queue = asyncio.Queue()
|
||||
|
||||
async def inject_cb():
|
||||
items = []
|
||||
while not injection_queue.empty():
|
||||
items.append(await injection_queue.get())
|
||||
return items
|
||||
|
||||
# Inject a follow-up that arrives during the first response
|
||||
await injection_queue.put(
|
||||
InboundMessage(channel="cli", sender_id="u", chat_id="c", content="quick follow-up")
|
||||
)
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "hello"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=5,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
hook=TrackingHook(),
|
||||
injection_callback=inject_cb,
|
||||
))
|
||||
|
||||
assert result.had_injections is True
|
||||
assert result.final_content == "second answer"
|
||||
assert call_count["n"] == 2
|
||||
# First stream_end should have resuming=True (because injections found)
|
||||
assert stream_end_calls[0] is True
|
||||
# Second (final) stream_end should have resuming=False
|
||||
assert stream_end_calls[-1] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_injection_cycles_capped_at_max():
|
||||
"""Injection cycles should be capped at _MAX_INJECTION_CYCLES."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner, _MAX_INJECTION_CYCLES
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
provider = MagicMock()
|
||||
call_count = {"n": 0}
|
||||
|
||||
async def chat_with_retry(*, messages, **kwargs):
|
||||
call_count["n"] += 1
|
||||
return LLMResponse(content=f"answer-{call_count['n']}", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_with_retry = chat_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
drain_count = {"n": 0}
|
||||
|
||||
async def inject_cb():
|
||||
drain_count["n"] += 1
|
||||
# Only inject for the first _MAX_INJECTION_CYCLES drains
|
||||
if drain_count["n"] <= _MAX_INJECTION_CYCLES:
|
||||
return [InboundMessage(channel="cli", sender_id="u", chat_id="c", content=f"msg-{drain_count['n']}")]
|
||||
return []
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "start"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=20,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
injection_callback=inject_cb,
|
||||
))
|
||||
|
||||
assert result.had_injections is True
|
||||
# Should be capped: _MAX_INJECTION_CYCLES injection rounds + 1 final round
|
||||
assert call_count["n"] == _MAX_INJECTION_CYCLES + 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_injections_flag_is_false_by_default():
|
||||
"""had_injections should be False when no injection callback or no messages."""
|
||||
from nanobot.agent.runner import AgentRunSpec, AgentRunner
|
||||
|
||||
provider = MagicMock()
|
||||
|
||||
async def chat_with_retry(**kwargs):
|
||||
return LLMResponse(content="done", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_with_retry = chat_with_retry
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[{"role": "user", "content": "hi"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=1,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
))
|
||||
|
||||
assert result.had_injections is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pending_queue_cleanup_on_dispatch(tmp_path):
|
||||
"""_pending_queues should be cleaned up after _dispatch completes."""
|
||||
loop = _make_loop(tmp_path)
|
||||
|
||||
async def chat_with_retry(**kwargs):
|
||||
return LLMResponse(content="done", tool_calls=[], usage={})
|
||||
|
||||
loop.provider.chat_with_retry = chat_with_retry
|
||||
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
msg = InboundMessage(channel="cli", sender_id="u", chat_id="c", content="hello")
|
||||
# The queue should not exist before dispatch
|
||||
assert msg.session_key not in loop._pending_queues
|
||||
|
||||
await loop._dispatch(msg)
|
||||
|
||||
# The queue should be cleaned up after dispatch
|
||||
assert msg.session_key not in loop._pending_queues
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_followup_routed_to_pending_queue(tmp_path):
|
||||
"""When a session has an active dispatch, follow-up messages go to pending queue."""
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop = _make_loop(tmp_path)
|
||||
|
||||
# Simulate an active dispatch by manually adding a pending queue
|
||||
pending = asyncio.Queue(maxsize=20)
|
||||
loop._pending_queues["cli:c"] = pending
|
||||
|
||||
msg = InboundMessage(channel="cli", sender_id="u", chat_id="c", content="follow-up")
|
||||
|
||||
# Directly test the routing logic from run() — if session_key is in
|
||||
# _pending_queues, the message should be put into the queue.
|
||||
assert msg.session_key in loop._pending_queues
|
||||
loop._pending_queues[msg.session_key].put_nowait(msg)
|
||||
|
||||
assert not pending.empty()
|
||||
queued_msg = pending.get_nowait()
|
||||
assert queued_msg.content == "follow-up"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_republishes_leftover_queue_messages(tmp_path):
|
||||
"""Messages left in the pending queue after _dispatch are re-published to the bus.
|
||||
|
||||
This tests the finally-block cleanup that prevents message loss when
|
||||
the runner exits early (e.g., max_iterations, tool_error) with messages
|
||||
still in the queue.
|
||||
"""
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop = _make_loop(tmp_path)
|
||||
bus = loop.bus
|
||||
|
||||
# Simulate a completed dispatch by manually registering a queue
|
||||
# with leftover messages, then running the cleanup logic directly.
|
||||
pending = asyncio.Queue(maxsize=20)
|
||||
session_key = "cli:c"
|
||||
loop._pending_queues[session_key] = pending
|
||||
pending.put_nowait(InboundMessage(channel="cli", sender_id="u", chat_id="c", content="leftover-1"))
|
||||
pending.put_nowait(InboundMessage(channel="cli", sender_id="u", chat_id="c", content="leftover-2"))
|
||||
|
||||
# Execute the cleanup logic from the finally block
|
||||
queue = loop._pending_queues.pop(session_key, None)
|
||||
assert queue is not None
|
||||
leftover = 0
|
||||
while True:
|
||||
try:
|
||||
item = queue.get_nowait()
|
||||
except asyncio.QueueEmpty:
|
||||
break
|
||||
await bus.publish_inbound(item)
|
||||
leftover += 1
|
||||
|
||||
assert leftover == 2
|
||||
|
||||
# Verify the messages are now on the bus
|
||||
msgs = []
|
||||
while not bus.inbound.empty():
|
||||
msgs.append(await asyncio.wait_for(bus.consume_inbound(), timeout=0.5))
|
||||
contents = [m.content for m in msgs]
|
||||
assert "leftover-1" in contents
|
||||
assert "leftover-2" in contents
|
||||
|
||||
@@ -107,7 +107,7 @@ class TestMessageToolSuppressLogic:
|
||||
async def on_progress(content: str, *, tool_hint: bool = False) -> None:
|
||||
progress.append((content, tool_hint))
|
||||
|
||||
final_content, _, _, _ = await loop._run_agent_loop([], on_progress=on_progress)
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop([], on_progress=on_progress)
|
||||
|
||||
assert final_content == "Done"
|
||||
assert progress == [
|
||||
|
||||
Reference in New Issue
Block a user