fix(webui): deliver late subagent results as new turns (#4992)
This commit is contained in:
@@ -5,7 +5,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnKind, TurnRoute, TurnState
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnKind, TurnState
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.schema import ChannelsConfig
|
||||
@@ -42,20 +42,21 @@ async def test_state_restore_extracts_documents_by_default(
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.loop.extract_documents", fake_extract_documents)
|
||||
|
||||
msg = InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="u",
|
||||
chat_id="c",
|
||||
content="summarize",
|
||||
media=[str(doc_path)],
|
||||
)
|
||||
ctx = TurnContext(
|
||||
msg=InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="u",
|
||||
chat_id="c",
|
||||
content="summarize",
|
||||
media=[str(doc_path)],
|
||||
),
|
||||
msg=msg,
|
||||
session_key="cli:c",
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
route=TurnRoute(channel="cli", chat_id="c"),
|
||||
delivery=loop.turn_delivery_factory.create(msg, "cli:c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
@@ -79,20 +80,21 @@ async def test_state_restore_references_documents_when_extraction_disabled(
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.loop.extract_documents", fail_extract_documents)
|
||||
|
||||
msg = InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="u",
|
||||
chat_id="c",
|
||||
content="summarize",
|
||||
media=[str(doc_path)],
|
||||
)
|
||||
ctx = TurnContext(
|
||||
msg=InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="u",
|
||||
chat_id="c",
|
||||
content="summarize",
|
||||
media=[str(doc_path)],
|
||||
),
|
||||
msg=msg,
|
||||
session_key="cli:c",
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
route=TurnRoute(channel="cli", chat_id="c"),
|
||||
delivery=loop.turn_delivery_factory.create(msg, "cli:c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
|
||||
@@ -8,6 +8,7 @@ import pytest
|
||||
|
||||
from nanobot.agent.hooks import create_file_edit_activity_hook
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.agent.tools.context import current_request_context
|
||||
from nanobot.agent.tools.filesystem import WriteFileTool
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.outbound_events import (
|
||||
@@ -22,11 +23,12 @@ from nanobot.bus.outbound_events import (
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.providers.base import LLMResponse, ToolCallRequest
|
||||
from nanobot.providers.factory import ProviderSnapshot
|
||||
from nanobot.session.webui_turns import WebuiTurnCoordinator
|
||||
from nanobot.session.webui_turns import WebuiTurnCoordinator, WebuiTurnRoutePolicy
|
||||
from nanobot.utils.progress_events import (
|
||||
invoke_file_edit_progress,
|
||||
on_progress_accepts_file_edit_events,
|
||||
)
|
||||
from nanobot.webui.metadata import WEBUI_TURN_METADATA_KEY
|
||||
|
||||
|
||||
def _make_loop(tmp_path: Path) -> AgentLoop:
|
||||
@@ -43,6 +45,7 @@ def _make_loop(tmp_path: Path) -> AgentLoop:
|
||||
|
||||
|
||||
def _attach_webui_runtime_events(loop: AgentLoop, bus: MessageBus) -> None:
|
||||
loop.turn_delivery_factory.route_policy = WebuiTurnRoutePolicy(loop.sessions)
|
||||
coordinator = WebuiTurnCoordinator(
|
||||
bus=bus,
|
||||
sessions=loop.sessions,
|
||||
@@ -347,12 +350,14 @@ class TestToolEventProgress:
|
||||
"status": "editing",
|
||||
}]
|
||||
|
||||
progress = await loop._build_bus_progress_callback(InboundMessage(
|
||||
msg = InboundMessage(
|
||||
channel="telegram",
|
||||
sender_id="u1",
|
||||
chat_id="chat1",
|
||||
content="edit",
|
||||
))
|
||||
)
|
||||
progress = loop.turn_delivery_factory.create(msg, msg.session_key).progress_callback()
|
||||
assert progress is not None
|
||||
assert on_progress_accepts_file_edit_events(progress) is True
|
||||
await invoke_file_edit_progress(progress, edit_events)
|
||||
outbound = await bus.consume_outbound()
|
||||
@@ -529,6 +534,157 @@ class TestToolEventProgress:
|
||||
assert turn_end_msgs[0].content == ""
|
||||
provider.chat_with_retry.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_independent_late_subagent_result_gets_complete_webui_turn(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
provider.get_default_model.return_value = "openai-codex/gpt-5.5"
|
||||
first_request_started = asyncio.Event()
|
||||
release_first_request = asyncio.Event()
|
||||
requests: list[list[dict]] = []
|
||||
request_contexts = []
|
||||
tool_call = ToolCallRequest(id="call1", name="custom_tool", arguments={})
|
||||
responses = iter([
|
||||
LLMResponse(content="Checking", tool_calls=[tool_call]),
|
||||
LLMResponse(content="The late result is ready", tool_calls=[]),
|
||||
])
|
||||
|
||||
async def chat_stream_with_retry(*, messages, on_content_delta, **kwargs):
|
||||
requests.append([dict(message) for message in messages])
|
||||
response = next(responses)
|
||||
if len(requests) == 1:
|
||||
first_request_started.set()
|
||||
await release_first_request.wait()
|
||||
await on_content_delta(response.content or "")
|
||||
return response
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
provider.chat_with_retry = AsyncMock()
|
||||
loop = AgentLoop(
|
||||
bus=bus,
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
model="openai-codex/gpt-5.5",
|
||||
)
|
||||
_attach_webui_runtime_events(loop, bus)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
loop.tools.prepare_call = MagicMock(return_value=(None, {}, None))
|
||||
|
||||
async def execute_tool(*args, **kwargs):
|
||||
request_contexts.append(current_request_context())
|
||||
return "ok"
|
||||
|
||||
loop.tools.execute = execute_tool
|
||||
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock( # type: ignore[method-assign]
|
||||
return_value=False
|
||||
)
|
||||
|
||||
session_key = "websocket:chat-a"
|
||||
session = loop.sessions.get_or_create(session_key)
|
||||
session.add_message("user", "Run this in the background")
|
||||
session.metadata.update({"webui": True, "title": "Existing title"})
|
||||
loop.sessions.save(session)
|
||||
dispatch = asyncio.create_task(loop._dispatch(InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id=session_key,
|
||||
content="Background research completed",
|
||||
session_key_override=session_key,
|
||||
metadata={
|
||||
"injected_event": "subagent_result",
|
||||
"subagent_task_id": "sub-1",
|
||||
},
|
||||
)))
|
||||
|
||||
await asyncio.wait_for(first_request_started.wait(), timeout=1)
|
||||
await loop._pending_queues[session_key].put(InboundMessage(
|
||||
channel="websocket",
|
||||
sender_id="user",
|
||||
chat_id="chat-a",
|
||||
content="Can you include the key detail?",
|
||||
session_key_override=session_key,
|
||||
))
|
||||
release_first_request.set()
|
||||
await asyncio.wait_for(dispatch, timeout=2)
|
||||
|
||||
outbound = []
|
||||
while bus.outbound_size > 0:
|
||||
outbound.append(await bus.consume_outbound())
|
||||
|
||||
assert len(requests) == 2
|
||||
assert requests[0][-1]["role"] == "user"
|
||||
assert requests[0][-1]["content"].endswith("Background research completed")
|
||||
assert any(
|
||||
message.get("role") == "user"
|
||||
and message.get("content") == "Can you include the key detail?"
|
||||
for message in requests[1]
|
||||
)
|
||||
assert len(request_contexts) == 1
|
||||
request_ctx = request_contexts[0]
|
||||
assert request_ctx is not None
|
||||
assert request_ctx.metadata == {
|
||||
"injected_event": "subagent_result",
|
||||
"subagent_task_id": "sub-1",
|
||||
}
|
||||
statuses = [
|
||||
message.event.status
|
||||
for message in outbound
|
||||
if isinstance(message.event, GoalStatusEvent)
|
||||
]
|
||||
assert statuses == ["running", "idle"]
|
||||
assert [
|
||||
message.content
|
||||
for message in outbound
|
||||
if isinstance(message.event, StreamDeltaEvent)
|
||||
] == ["Checking", "The late result is ready"]
|
||||
assert any(isinstance(message.event, ProgressEvent) for message in outbound)
|
||||
assert len([
|
||||
message for message in outbound if isinstance(message.event, TurnEndEvent)
|
||||
]) == 1
|
||||
assert len([
|
||||
message for message in outbound
|
||||
if isinstance(message.event, StreamedResponseEvent)
|
||||
]) == 1
|
||||
visible_events = [
|
||||
message
|
||||
for message in outbound
|
||||
if isinstance(
|
||||
message.event,
|
||||
GoalStatusEvent
|
||||
| ProgressEvent
|
||||
| StreamDeltaEvent
|
||||
| StreamEndEvent
|
||||
| StreamedResponseEvent
|
||||
| TurnEndEvent,
|
||||
)
|
||||
]
|
||||
assert visible_events
|
||||
turn_ids = {
|
||||
message.metadata.get(WEBUI_TURN_METADATA_KEY)
|
||||
for message in visible_events
|
||||
}
|
||||
assert len(turn_ids) == 1
|
||||
turn_id = turn_ids.pop()
|
||||
assert isinstance(turn_id, str)
|
||||
assert turn_id.startswith("subagent:")
|
||||
assert all(
|
||||
(message.channel, message.chat_id) == ("websocket", "chat-a")
|
||||
and message.metadata.get("webui") is True
|
||||
and message.metadata.get("_wants_stream") is True
|
||||
and set(message.metadata) <= {
|
||||
"webui",
|
||||
"_wants_stream",
|
||||
WEBUI_TURN_METADATA_KEY,
|
||||
"latency_ms",
|
||||
}
|
||||
for message in visible_events
|
||||
)
|
||||
provider.chat_with_retry.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_timeout_recovery_continues_in_new_segment(
|
||||
self,
|
||||
|
||||
@@ -1424,6 +1424,7 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_
|
||||
# rewritten with volatile ``[Message Time: ...]`` prefixes.
|
||||
assert "[Message Time:" not in non_system[0]["content"]
|
||||
assert "[Message Time:" not in non_system[1]["content"]
|
||||
assert non_system[2]["role"] == "user"
|
||||
assert non_system[2]["content"].count("subagent result") == 1
|
||||
assert non_system[2]["content"] == "subagent result"
|
||||
|
||||
@@ -1580,10 +1581,11 @@ async def test_multiple_subagent_followups_all_persist_as_standalone_history(tmp
|
||||
]
|
||||
|
||||
|
||||
def test_prompt_merge_does_not_replace_standalone_subagent_history_entry(tmp_path: Path) -> None:
|
||||
def test_subagent_followup_uses_user_model_input_and_assistant_history(tmp_path: Path) -> None:
|
||||
loop = _mk_loop()
|
||||
session = Session(key="cli:merge")
|
||||
session.add_message("assistant", "previous assistant")
|
||||
history = session.get_history(max_messages=0)
|
||||
|
||||
inserted = loop._persist_subagent_followup(
|
||||
session,
|
||||
@@ -1600,16 +1602,18 @@ def test_prompt_merge_does_not_replace_standalone_subagent_history_entry(tmp_pat
|
||||
|
||||
builder = ContextBuilder(tmp_path)
|
||||
projected = builder.build_messages(
|
||||
history=session.get_history(max_messages=0),
|
||||
current_message="",
|
||||
current_role="assistant",
|
||||
history=history,
|
||||
current_message="subagent result",
|
||||
current_role="user",
|
||||
channel="cli",
|
||||
chat_id="merge",
|
||||
)
|
||||
|
||||
non_system = [m for m in projected if m.get("role") != "system"]
|
||||
assert len(non_system) == 2
|
||||
assert non_system[-1]["role"] == "user"
|
||||
assert "subagent result" in non_system[-1]["content"]
|
||||
assert session.messages[-1]["role"] == "assistant"
|
||||
assert session.messages[-1]["content"] == "subagent result"
|
||||
assert session.messages[-1]["injected_event"] == "subagent_result"
|
||||
|
||||
|
||||
@@ -689,6 +689,8 @@ async def test_waiting_dispatch_does_not_replace_active_pending_queue(tmp_path):
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop = _make_loop(tmp_path)
|
||||
route_policy = MagicMock(side_effect=lambda _msg, _key, route: route)
|
||||
loop.turn_delivery_factory.route_policy = route_policy
|
||||
session_key = "cli:c"
|
||||
lock = loop._session_locks.setdefault(session_key, asyncio.Lock())
|
||||
await lock.acquire()
|
||||
@@ -712,10 +714,12 @@ async def test_waiting_dispatch_does_not_replace_active_pending_queue(tmp_path):
|
||||
await asyncio.wait_for(waiting_at_lock.wait(), timeout=2.0)
|
||||
|
||||
assert loop._pending_queues[session_key] is active_pending
|
||||
route_policy.assert_not_called()
|
||||
|
||||
waiting.cancel()
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await waiting
|
||||
route_policy.assert_not_called()
|
||||
lock.release()
|
||||
|
||||
|
||||
@@ -746,6 +750,44 @@ async def test_followup_routed_to_pending_queue(tmp_path):
|
||||
assert queued_msg.session_key == UNIFIED_SESSION_KEY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mid_turn_subagent_result_does_not_resolve_a_new_turn_route(tmp_path):
|
||||
"""Injected results stay inside the active turn instead of opening a side turn."""
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop = _make_loop(tmp_path)
|
||||
loop._dispatch = AsyncMock() # type: ignore[method-assign]
|
||||
route_policy = MagicMock(side_effect=lambda _msg, _key, route: route)
|
||||
loop.turn_delivery_factory.route_policy = route_policy
|
||||
|
||||
session_key = "websocket:chat-1"
|
||||
pending = asyncio.Queue(maxsize=20)
|
||||
loop._pending_queues[session_key] = pending
|
||||
|
||||
run_task = asyncio.create_task(loop.run())
|
||||
msg = InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id=session_key,
|
||||
content="background result",
|
||||
metadata={
|
||||
"injected_event": "subagent_result",
|
||||
"subagent_task_id": "sub-1",
|
||||
},
|
||||
session_key_override=session_key,
|
||||
)
|
||||
await loop.bus.publish_inbound(msg)
|
||||
|
||||
queued_msg = await asyncio.wait_for(pending.get(), timeout=2)
|
||||
|
||||
loop.stop()
|
||||
await asyncio.wait_for(run_task, timeout=2)
|
||||
|
||||
assert queued_msg is msg
|
||||
assert loop._dispatch.await_count == 0
|
||||
route_policy.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cron_turn_deferred_while_session_active(tmp_path):
|
||||
"""Cron turns wait for the active session instead of becoming injections."""
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
from pathlib import Path
|
||||
|
||||
from nanobot.agent.turn_delivery import TurnDeliveryFactory
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.bus.runtime_events import RuntimeEventBus
|
||||
from nanobot.session.manager import SessionManager
|
||||
from nanobot.session.webui_turns import WebuiTurnRoutePolicy
|
||||
from nanobot.webui.metadata import WEBUI_TURN_METADATA_KEY
|
||||
|
||||
|
||||
def test_late_subagent_route_requires_webui_owned_session(tmp_path: Path) -> None:
|
||||
sessions = SessionManager(tmp_path)
|
||||
factory = TurnDeliveryFactory(
|
||||
MessageBus(),
|
||||
RuntimeEventBus(),
|
||||
route_policy=WebuiTurnRoutePolicy(sessions),
|
||||
)
|
||||
session_key = "websocket:chat-a"
|
||||
msg = InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id=session_key,
|
||||
content="Background research completed",
|
||||
session_key_override=session_key,
|
||||
metadata={
|
||||
"injected_event": "subagent_result",
|
||||
"subagent_task_id": "sub-1",
|
||||
},
|
||||
)
|
||||
|
||||
hidden_route = factory.create(msg, session_key).route
|
||||
|
||||
assert hidden_route.channel == "websocket"
|
||||
assert hidden_route.chat_id == "chat-a"
|
||||
assert hidden_route.metadata == {}
|
||||
assert hidden_route.publish_lifecycle is False
|
||||
|
||||
session = sessions.get_or_create(session_key)
|
||||
session.metadata["webui"] = True
|
||||
first_visible_route = factory.create(msg, session_key).route
|
||||
second_visible_route = factory.create(msg, session_key).route
|
||||
|
||||
assert first_visible_route.publish_lifecycle is True
|
||||
assert set(first_visible_route.metadata) == {
|
||||
"webui",
|
||||
"_wants_stream",
|
||||
WEBUI_TURN_METADATA_KEY,
|
||||
}
|
||||
assert first_visible_route.metadata["webui"] is True
|
||||
assert first_visible_route.metadata["_wants_stream"] is True
|
||||
first_turn_id = first_visible_route.metadata[WEBUI_TURN_METADATA_KEY]
|
||||
second_turn_id = second_visible_route.metadata[WEBUI_TURN_METADATA_KEY]
|
||||
assert first_turn_id.startswith("subagent:")
|
||||
assert second_turn_id.startswith("subagent:")
|
||||
assert first_turn_id != second_turn_id
|
||||
assert msg.metadata == {
|
||||
"injected_event": "subagent_result",
|
||||
"subagent_task_id": "sub-1",
|
||||
}
|
||||
Reference in New Issue
Block a user