refactor(agent): remove dead lifecycle scaffolding
This commit is contained in:
@@ -353,6 +353,14 @@ class TestBuildSystemPrompt:
|
||||
|
||||
|
||||
class TestBuildMessages:
|
||||
def test_optional_arguments_are_keyword_only(self, tmp_path):
|
||||
builder = _builder(tmp_path)
|
||||
|
||||
with pytest.raises(TypeError):
|
||||
builder.build_system_prompt(["legacy-skill"])
|
||||
with pytest.raises(TypeError):
|
||||
builder.build_messages([], "hello", ["legacy-skill"])
|
||||
|
||||
def test_basic_empty_history(self, tmp_path):
|
||||
builder = _builder(tmp_path)
|
||||
messages = builder.build_messages([], "hello")
|
||||
@@ -363,7 +371,7 @@ class TestBuildMessages:
|
||||
|
||||
def test_runtime_context_is_not_injected_by_default(self, tmp_path):
|
||||
builder = _builder(tmp_path)
|
||||
messages = builder.build_messages([], "hello", channel="cli", chat_id="direct")
|
||||
messages = builder.build_messages([], "hello", channel="cli")
|
||||
user_msg = str(messages[-1]["content"])
|
||||
assert user_msg == "hello"
|
||||
|
||||
|
||||
@@ -70,7 +70,6 @@ def test_provider_context_appended_after_user_content(tmp_path) -> None:
|
||||
history=[],
|
||||
current_message="hello world",
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
runtime_context_blocks=[
|
||||
RuntimeContextBlock(source="test", content="provider context"),
|
||||
],
|
||||
@@ -322,7 +321,7 @@ def test_build_messages_passes_channel_to_system_prompt(tmp_path) -> None:
|
||||
|
||||
messages = builder.build_messages(
|
||||
history=[], current_message="hi",
|
||||
channel="telegram", chat_id="123",
|
||||
channel="telegram",
|
||||
)
|
||||
system = messages[0]["content"]
|
||||
assert "Format Hint" in system
|
||||
@@ -349,7 +348,6 @@ def test_subagent_result_does_not_create_consecutive_assistant_messages(tmp_path
|
||||
history=[{"role": "assistant", "content": "previous result"}],
|
||||
current_message="subagent result",
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
current_role="assistant",
|
||||
)
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnKind, TurnState
|
||||
from nanobot.agent.loop import AgentLoop, TurnContext, TurnKind
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.schema import ChannelsConfig
|
||||
@@ -27,7 +27,7 @@ def _make_loop(tmp_path: Path, channels_config: ChannelsConfig | None = None) ->
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_state_restore_extracts_documents_by_default(
|
||||
async def test_restore_turn_extracts_documents_by_default(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
@@ -52,14 +52,13 @@ async def test_state_restore_extracts_documents_by_default(
|
||||
ctx = TurnContext(
|
||||
msg=msg,
|
||||
session_key="cli:c",
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
delivery=loop.turn_delivery_factory.create(msg, "cli:c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
await loop._restore_turn(ctx)
|
||||
|
||||
assert calls == [("summarize", [str(doc_path)])]
|
||||
assert "Quarterly revenue" in ctx.msg.content
|
||||
@@ -67,7 +66,7 @@ async def test_state_restore_extracts_documents_by_default(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_state_restore_references_documents_when_extraction_disabled(
|
||||
async def test_restore_turn_references_documents_when_extraction_disabled(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
@@ -90,14 +89,13 @@ async def test_state_restore_references_documents_when_extraction_disabled(
|
||||
ctx = TurnContext(
|
||||
msg=msg,
|
||||
session_key="cli:c",
|
||||
state=TurnState.RESTORE,
|
||||
turn_id="turn-1",
|
||||
runtime=loop.llm_runtime(),
|
||||
kind=TurnKind.USER,
|
||||
delivery=loop.turn_delivery_factory.create(msg, "cli:c"),
|
||||
)
|
||||
|
||||
assert await loop._state_restore(ctx) == "ok"
|
||||
await loop._restore_turn(ctx)
|
||||
|
||||
assert "Quarterly revenue" not in ctx.msg.content
|
||||
assert f"[Attachment: {doc_path}]" in ctx.msg.content
|
||||
|
||||
@@ -414,13 +414,13 @@ class TestEphemeralDirect:
|
||||
|
||||
captured = {}
|
||||
|
||||
original_save = loop._state_save
|
||||
original_save = loop._persist_turn
|
||||
|
||||
async def patched_save(ctx):
|
||||
captured["ephemeral"] = ctx.ephemeral
|
||||
return await original_save(ctx)
|
||||
|
||||
with patch.object(loop, "_state_save", side_effect=patched_save):
|
||||
with patch.object(loop, "_persist_turn", side_effect=patched_save):
|
||||
await loop.process_direct(
|
||||
"test", session_key="dream:check", ephemeral=True,
|
||||
)
|
||||
@@ -435,13 +435,13 @@ class TestEphemeralDirect:
|
||||
|
||||
captured = {}
|
||||
|
||||
original_save = loop._state_save
|
||||
original_save = loop._persist_turn
|
||||
|
||||
async def patched_save(ctx):
|
||||
captured["ephemeral"] = ctx.ephemeral
|
||||
return await original_save(ctx)
|
||||
|
||||
with patch.object(loop, "_state_save", side_effect=patched_save):
|
||||
with patch.object(loop, "_persist_turn", side_effect=patched_save):
|
||||
await loop.process_direct("test", session_key="cli:normal")
|
||||
|
||||
assert captured.get("ephemeral") is False
|
||||
|
||||
@@ -7,7 +7,7 @@ import pytest
|
||||
from loguru import logger
|
||||
|
||||
from nanobot.agent.context import ContextBuilder
|
||||
from nanobot.agent.loop import AgentLoop, TurnState
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.agent.tools.context import RequestContext, request_context
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.outbound_events import (
|
||||
@@ -451,7 +451,6 @@ def test_build_and_save_preserves_user_text_containing_goal_guidance_tag(tmp_pat
|
||||
[],
|
||||
user_text,
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
)
|
||||
assert "_meta" not in messages[-1]
|
||||
|
||||
@@ -476,7 +475,6 @@ def test_build_and_save_preserves_multimodal_user_block_starting_with_runtime_ta
|
||||
user_text,
|
||||
media=[str(image)],
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
)
|
||||
|
||||
loop._save_turn(session, messages, skip=1)
|
||||
@@ -1101,7 +1099,7 @@ async def test_websocket_internal_continuation_keeps_single_visible_run(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_message_uses_context_chat_id_for_runtime_prompt(tmp_path: Path) -> None:
|
||||
async def test_process_message_keeps_delivery_chat_for_thread_session(tmp_path: Path) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign]
|
||||
loop.context.build_messages = MagicMock( # type: ignore[method-assign]
|
||||
@@ -1135,12 +1133,11 @@ async def test_process_message_uses_context_chat_id_for_runtime_prompt(tmp_path:
|
||||
|
||||
assert result is not None
|
||||
assert result.chat_id == "thread-777"
|
||||
assert loop.context.build_messages.call_args.kwargs["chat_id"] == "parent-456"
|
||||
assert loop._run_agent_loop.call_args.kwargs["chat_id"] == "thread-777"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_message_uses_explicit_session_metadata_for_goal_context(
|
||||
async def test_process_message_uses_explicit_session_for_goal_context(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
@@ -1185,10 +1182,10 @@ async def test_process_message_uses_explicit_session_metadata_for_goal_context(
|
||||
|
||||
assert result is not None
|
||||
assert result.content == "ok"
|
||||
kwargs = loop.context.build_messages.call_args.kwargs
|
||||
assert kwargs["chat_id"] == "chat-with-goal"
|
||||
assert kwargs["session_metadata"] is system_session.metadata
|
||||
assert GOAL_STATE_KEY not in kwargs["session_metadata"]
|
||||
kwargs = loop._run_agent_loop.call_args.kwargs
|
||||
assert kwargs["session"] is system_session
|
||||
assert kwargs["session_key"] == "system"
|
||||
assert GOAL_STATE_KEY not in kwargs["session"].metadata
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -1570,27 +1567,26 @@ async def test_system_subagent_followup_does_not_log_content(tmp_path: Path) ->
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_subagent_followup_uses_common_turn_state_machine(tmp_path: Path) -> None:
|
||||
async def test_system_subagent_followup_uses_common_turn_lifecycle(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] = []
|
||||
visited: list[str] = []
|
||||
|
||||
for state in (
|
||||
TurnState.RESTORE,
|
||||
TurnState.COMPACT,
|
||||
TurnState.COMMAND,
|
||||
TurnState.BUILD,
|
||||
TurnState.RUN,
|
||||
TurnState.SAVE,
|
||||
TurnState.RESPOND,
|
||||
for name in (
|
||||
"_restore_turn",
|
||||
"_compact_session",
|
||||
"_dispatch_command",
|
||||
"_build_turn",
|
||||
"_run_turn",
|
||||
"_persist_turn",
|
||||
"_prepare_outbound",
|
||||
):
|
||||
name = f"_state_{state.name.lower()}"
|
||||
original = getattr(loop, name)
|
||||
|
||||
async def record(ctx, *, _original=original, _state=state):
|
||||
visited.append(_state)
|
||||
async def record(ctx, *, _original=original, _name=name):
|
||||
visited.append(_name)
|
||||
return await _original(ctx)
|
||||
|
||||
setattr(loop, name, record)
|
||||
@@ -1606,25 +1602,33 @@ async def test_system_subagent_followup_uses_common_turn_state_machine(tmp_path:
|
||||
|
||||
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"},
|
||||
logs: list[str] = []
|
||||
sink_id = logger.add(logs.append, level="DEBUG", format="{message}")
|
||||
try:
|
||||
await loop._process_message(
|
||||
InboundMessage(
|
||||
channel="system",
|
||||
sender_id="subagent",
|
||||
chat_id="cli:test",
|
||||
content="subagent result",
|
||||
metadata={"subagent_task_id": "sub-1"},
|
||||
)
|
||||
)
|
||||
)
|
||||
finally:
|
||||
logger.remove(sink_id)
|
||||
|
||||
assert visited == [
|
||||
TurnState.RESTORE,
|
||||
TurnState.COMPACT,
|
||||
TurnState.COMMAND,
|
||||
TurnState.BUILD,
|
||||
TurnState.RUN,
|
||||
TurnState.SAVE,
|
||||
TurnState.RESPOND,
|
||||
"_restore_turn",
|
||||
"_compact_session",
|
||||
"_dispatch_command",
|
||||
"_build_turn",
|
||||
"_run_turn",
|
||||
"_persist_turn",
|
||||
"_prepare_outbound",
|
||||
]
|
||||
logged = "".join(logs)
|
||||
for stage in ("restore", "compact", "command", "build", "run", "save", "respond"):
|
||||
assert f"Stage {stage} completed in" in logged
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -1689,7 +1693,6 @@ def test_subagent_followup_uses_user_model_input_and_assistant_history(tmp_path:
|
||||
current_message="subagent result",
|
||||
current_role="user",
|
||||
channel="cli",
|
||||
chat_id="merge",
|
||||
)
|
||||
|
||||
non_system = [m for m in projected if m.get("role") != "system"]
|
||||
|
||||
@@ -225,7 +225,7 @@ async def test_process_message_captures_original_text_before_restore(
|
||||
seen.append((ctx.original_user_text, ctx.runtime))
|
||||
raise RuntimeError("captured before restore")
|
||||
|
||||
loop._state_restore = stop_after_capture # type: ignore[method-assign]
|
||||
loop._restore_turn = stop_after_capture # type: ignore[method-assign]
|
||||
|
||||
with pytest.raises(RuntimeError, match="captured before restore"):
|
||||
await loop._process_message(
|
||||
|
||||
@@ -135,7 +135,7 @@ async def test_tool_fails_after_retry_exhausted():
|
||||
|
||||
assert "failed after retry" in output
|
||||
assert "ClosedResourceError" in output
|
||||
assert is_tool_error_result(wrapper.name, output)
|
||||
assert is_tool_error_result(output)
|
||||
assert session.call_tool.call_count == 2
|
||||
|
||||
|
||||
|
||||
@@ -140,5 +140,5 @@ async def test_loader_entry_point_error_wrapper_preserves_tool_api(tmp_path):
|
||||
assert tool.to_schema() == {"name": "api_plugin", "custom": True}
|
||||
|
||||
result = await tool.execute(value="1")
|
||||
assert is_tool_error_result("api_plugin", result) is True
|
||||
assert is_tool_error_result(result) is True
|
||||
assert str(result) == "Error: plugin failed"
|
||||
|
||||
@@ -78,7 +78,7 @@ async def test_run_inline_returns_structured_error(tmp_path):
|
||||
)
|
||||
|
||||
assert result == "subagent failed"
|
||||
assert is_tool_error_result("spawn", result)
|
||||
assert is_tool_error_result(result)
|
||||
assert manager._running_tasks == {}
|
||||
assert manager._session_tasks == {}
|
||||
|
||||
|
||||
@@ -380,7 +380,7 @@ def test_write_stdin_reports_missing_session(tmp_path):
|
||||
result = asyncio.run(tool.execute(session_id="missing\nExit code: 0", chars=""))
|
||||
|
||||
assert result == "Error: exec session not found: 'missing\\nExit code: 0'"
|
||||
assert is_tool_error_result("write_stdin", result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
def test_list_exec_sessions_reports_running_commands(tmp_path):
|
||||
|
||||
@@ -449,7 +449,7 @@ async def test_execute_wraps_mcp_is_error_result() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "Error: server-side MCP failure"
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -462,7 +462,7 @@ async def test_execute_contains_malformed_success_result() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "(MCP tool returned malformed content: TypeError)"
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -476,7 +476,7 @@ async def test_registry_adds_retry_hint_to_malformed_mcp_result() -> None:
|
||||
|
||||
result = await registry.execute(wrapper.name, {})
|
||||
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
assert "MCP tool returned malformed content" in result
|
||||
assert "Analyze the error above and try a different approach" in result
|
||||
|
||||
@@ -494,7 +494,7 @@ async def test_execute_preserves_success_text_that_starts_with_error() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "Error: generated report successfully"
|
||||
assert not is_tool_error_result(wrapper.name, result)
|
||||
assert not is_tool_error_result(result)
|
||||
|
||||
|
||||
# Smallest valid 1x1 PNG, base64 without the data: prefix.
|
||||
@@ -562,7 +562,7 @@ async def test_execute_returns_timeout_message() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "(MCP tool call timed out after 0.01s)"
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -575,7 +575,7 @@ async def test_execute_handles_server_cancelled_error() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "(MCP tool call was cancelled)"
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -607,7 +607,7 @@ async def test_execute_handles_generic_exception() -> None:
|
||||
result = await wrapper.execute()
|
||||
|
||||
assert result == "(MCP tool call failed: RuntimeError)"
|
||||
assert is_tool_error_result(wrapper.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
def _make_tool_def(name: str) -> SimpleNamespace:
|
||||
@@ -1631,7 +1631,7 @@ def test_long_server_name_tools_are_matched_by_server_name() -> None:
|
||||
assert wrapper._reconnect is not None
|
||||
assert other_wrapper._reconnect is None
|
||||
|
||||
removed = mcp_mod._unregister_server_tools(SimpleNamespace(), registry, server_name)
|
||||
removed = mcp_mod._unregister_server_tools(registry, server_name)
|
||||
|
||||
assert removed == 1
|
||||
assert wrapper.name not in registry.tool_names
|
||||
|
||||
@@ -60,7 +60,7 @@ class SampleTool(Tool):
|
||||
@tool_parameters(
|
||||
tool_parameters_schema(
|
||||
query=StringSchema(min_length=2),
|
||||
count=IntegerSchema(2, minimum=1, maximum=10),
|
||||
count=IntegerSchema(minimum=1, maximum=10),
|
||||
required=["query", "count"],
|
||||
)
|
||||
)
|
||||
@@ -81,12 +81,12 @@ def test_schema_validate_value_matches_tool_validate_params() -> None:
|
||||
"""ObjectSchema.validate_value 与 validate_json_schema_value、Tool.validate_params 一致。"""
|
||||
root = tool_parameters_schema(
|
||||
query=StringSchema(min_length=2),
|
||||
count=IntegerSchema(2, minimum=1, maximum=10),
|
||||
count=IntegerSchema(minimum=1, maximum=10),
|
||||
required=["query", "count"],
|
||||
)
|
||||
obj = ObjectSchema(
|
||||
query=StringSchema(min_length=2),
|
||||
count=IntegerSchema(2, minimum=1, maximum=10),
|
||||
count=IntegerSchema(minimum=1, maximum=10),
|
||||
required=["query", "count"],
|
||||
)
|
||||
params = {"query": "h", "count": 2}
|
||||
@@ -110,14 +110,14 @@ def test_schema_validate_value_matches_tool_validate_params() -> None:
|
||||
expected = _Mini().validate_params(params)
|
||||
assert Schema.validate_json_schema_value(params, root, "") == expected
|
||||
assert obj.validate_value(params, "") == expected
|
||||
assert IntegerSchema(0, minimum=1).validate_value(0, "n") == ["n must be >= 1"]
|
||||
assert IntegerSchema(minimum=1).validate_value(0, "n") == ["n must be >= 1"]
|
||||
|
||||
|
||||
def test_schema_classes_equivalent_to_sample_tool_parameters() -> None:
|
||||
"""Schema 类生成的 JSON Schema 应与手写 dict 一致,便于校验行为一致。"""
|
||||
built = tool_parameters_schema(
|
||||
query=StringSchema(min_length=2),
|
||||
count=IntegerSchema(2, minimum=1, maximum=10),
|
||||
count=IntegerSchema(minimum=1, maximum=10),
|
||||
mode=StringSchema("", enum=["fast", "full"]),
|
||||
meta=ObjectSchema(
|
||||
tag=StringSchema(""),
|
||||
|
||||
@@ -272,7 +272,7 @@ async def test_serper_search_http_error(monkeypatch):
|
||||
tool = _tool(provider="serper", api_key="bad-serper-key")
|
||||
result = await tool.execute(query="serper")
|
||||
assert "Error: Serper search failed (403)" in result
|
||||
assert is_tool_error_result(tool.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -284,7 +284,7 @@ async def test_serper_search_rate_limited(monkeypatch):
|
||||
tool = _tool(provider="serper", api_key="serper-key")
|
||||
result = await tool.execute(query="serper")
|
||||
assert "Serper search rate limited" in result
|
||||
assert is_tool_error_result(tool.name, result)
|
||||
assert is_tool_error_result(result)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user