refactor(agent): unify internal turn lifecycle (#4993)
This commit is contained in:
chengyongru
@@ -5,7 +5,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnState
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnKind, TurnRoute, TurnState
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.schema import ChannelsConfig
|
||||
@@ -54,6 +54,8 @@ async def test_state_restore_extracts_documents_by_default(
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
route=TurnRoute(channel="cli", chat_id="c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
@@ -89,6 +91,8 @@ async def test_state_restore_references_documents_when_extraction_disabled(
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
route=TurnRoute(channel="cli", chat_id="c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
|
||||
@@ -4,9 +4,10 @@ from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from loguru import logger
|
||||
|
||||
from nanobot.agent.context import ContextBuilder
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.agent.loop import AgentLoop, TurnState
|
||||
from nanobot.agent.tools.context import RequestContext, request_context
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.outbound_events import (
|
||||
@@ -1143,6 +1144,19 @@ async def test_run_agent_loop_goal_continue_message_reads_latest_metadata(
|
||||
assert "Goal created during this runner call." in (seen["goal_continue"] or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_direct_rejects_reserved_system_channel(tmp_path: Path) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
loop._connect_mcp = AsyncMock() # type: ignore[method-assign]
|
||||
loop._process_message = AsyncMock(return_value=None) # type: ignore[method-assign]
|
||||
|
||||
with pytest.raises(ValueError, match="reserved for internal messages"):
|
||||
await loop.process_direct("external input", channel="system")
|
||||
|
||||
loop._connect_mcp.assert_not_awaited()
|
||||
loop._process_message.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_direct_skip_user_persist_does_not_save_retry_user(
|
||||
tmp_path: Path,
|
||||
@@ -1363,6 +1377,7 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_
|
||||
async def fake_run_agent_loop(initial_messages, **kwargs):
|
||||
seen["initial_messages"] = initial_messages
|
||||
seen["runtime"] = kwargs["runtime"]
|
||||
seen["request_context"] = kwargs["request_context"]
|
||||
return (
|
||||
"done",
|
||||
[],
|
||||
@@ -1385,6 +1400,15 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_
|
||||
)
|
||||
|
||||
assert seen["runtime"] is runtime
|
||||
request = seen["request_context"]
|
||||
assert isinstance(request, RequestContext)
|
||||
assert request.channel == "cli"
|
||||
assert request.chat_id == "test"
|
||||
assert request.session_key == "cli:test"
|
||||
assert request.original_user_text is None
|
||||
assert request.sender_id == "subagent"
|
||||
assert request.metadata == {"subagent_task_id": "sub-1"}
|
||||
assert request.turn_id
|
||||
record_runtime.assert_called_once_with("cli:test", runtime)
|
||||
assert len(loop.consolidator.maybe_consolidate_by_tokens.call_args_list) == 2
|
||||
assert all(
|
||||
@@ -1421,6 +1445,104 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_subagent_followup_does_not_log_content(tmp_path: Path) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock( # type: ignore[method-assign]
|
||||
return_value=False
|
||||
)
|
||||
|
||||
async def fake_run_agent_loop(initial_messages, **_kwargs):
|
||||
return (
|
||||
"done",
|
||||
[],
|
||||
[*initial_messages, {"role": "assistant", "content": "done"}],
|
||||
"stop",
|
||||
False,
|
||||
)
|
||||
|
||||
loop._run_agent_loop = fake_run_agent_loop # type: ignore[method-assign]
|
||||
secret = "LEAKME42"
|
||||
content = f"[Subagent 'research' completed]\n\nTask: inspect logs\n\nResult:\n{secret}"
|
||||
logs: list[str] = []
|
||||
sink_id = logger.add(logs.append, level="INFO", format="{message}")
|
||||
|
||||
try:
|
||||
await loop._process_message(
|
||||
InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id="cli:logs",
|
||||
content=content,
|
||||
metadata={"subagent_task_id": "sub-logs"},
|
||||
)
|
||||
)
|
||||
finally:
|
||||
logger.remove(sink_id)
|
||||
|
||||
logged = "".join(logs)
|
||||
assert "Processing system message from subagent" in logged
|
||||
assert secret not in logged
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_subagent_followup_uses_common_turn_state_machine(tmp_path: Path) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock( # type: ignore[method-assign]
|
||||
return_value=False
|
||||
)
|
||||
visited: list[TurnState] = []
|
||||
|
||||
for state in (
|
||||
TurnState.RESTORE,
|
||||
TurnState.COMPACT,
|
||||
TurnState.COMMAND,
|
||||
TurnState.BUILD,
|
||||
TurnState.RUN,
|
||||
TurnState.SAVE,
|
||||
TurnState.RESPOND,
|
||||
):
|
||||
name = f"_state_{state.name.lower()}"
|
||||
original = getattr(loop, name)
|
||||
|
||||
async def record(ctx, *, _original=original, _state=state):
|
||||
visited.append(_state)
|
||||
return await _original(ctx)
|
||||
|
||||
setattr(loop, name, record)
|
||||
|
||||
async def fake_run_agent_loop(initial_messages, **_kwargs):
|
||||
return (
|
||||
"done",
|
||||
[],
|
||||
[*initial_messages, {"role": "assistant", "content": "done"}],
|
||||
"stop",
|
||||
False,
|
||||
)
|
||||
|
||||
loop._run_agent_loop = fake_run_agent_loop # type: ignore[method-assign]
|
||||
|
||||
await loop._process_message(
|
||||
InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id="cli:test",
|
||||
content="subagent result",
|
||||
metadata={"subagent_task_id": "sub-1"},
|
||||
)
|
||||
)
|
||||
|
||||
assert visited == [
|
||||
TurnState.RESTORE,
|
||||
TurnState.COMPACT,
|
||||
TurnState.COMMAND,
|
||||
TurnState.BUILD,
|
||||
TurnState.RUN,
|
||||
TurnState.SAVE,
|
||||
TurnState.RESPOND,
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_multiple_subagent_followups_all_persist_as_standalone_history(tmp_path: Path) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
@@ -1556,10 +1678,11 @@ async def test_system_subagent_followup_uses_thread_session_and_slack_metadata(t
|
||||
thread_session.add_message("user", "thread question")
|
||||
loop.sessions.save(thread_session)
|
||||
|
||||
seen: dict[str, list[dict]] = {}
|
||||
seen: dict[str, object] = {}
|
||||
|
||||
async def fake_run_agent_loop(initial_messages, **_kwargs):
|
||||
async def fake_run_agent_loop(initial_messages, **kwargs):
|
||||
seen["initial_messages"] = initial_messages
|
||||
seen["request_context"] = kwargs["request_context"]
|
||||
return (
|
||||
"done",
|
||||
[],
|
||||
@@ -1588,7 +1711,18 @@ async def test_system_subagent_followup_uses_thread_session_and_slack_metadata(t
|
||||
"slack": {"thread_ts": "1700.42"},
|
||||
"origin_message_id": "msg-123",
|
||||
}
|
||||
assert "thread question" in seen["initial_messages"][1]["content"]
|
||||
request = seen["request_context"]
|
||||
assert isinstance(request, RequestContext)
|
||||
assert request.channel == "slack"
|
||||
assert request.chat_id == "C123"
|
||||
assert request.metadata == {
|
||||
"subagent_task_id": "sub-1",
|
||||
"origin_message_id": "msg-123",
|
||||
}
|
||||
assert "slack" not in request.metadata
|
||||
initial_messages = seen["initial_messages"]
|
||||
assert isinstance(initial_messages, list)
|
||||
assert "thread question" in initial_messages[1]["content"]
|
||||
|
||||
loop.sessions.invalidate("slack:C123:1700.42")
|
||||
persisted = loop.sessions.get_or_create("slack:C123:1700.42")
|
||||
|
||||
Reference in New Issue
Block a user