From 39348dfafe7c6010e1cdc22faf68eeaa363b929e Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 27 Jul 2026 11:52:14 +0800 Subject: [PATCH] refactor(agent): remove dead lifecycle scaffolding --- nanobot/agent/context.py | 8 +- nanobot/agent/loop.py | 168 ++++++------------ nanobot/agent/memory.py | 5 +- nanobot/agent/runner.py | 2 +- nanobot/agent/tools/apply_patch.py | 6 - nanobot/agent/tools/cron.py | 4 +- nanobot/agent/tools/exec_session.py | 4 - nanobot/agent/tools/filesystem.py | 6 - nanobot/agent/tools/mcp.py | 6 +- nanobot/agent/tools/registry.py | 4 +- nanobot/agent/tools/schema.py | 6 +- nanobot/agent/tools/shell.py | 1 - nanobot/agent/tools/web.py | 5 +- tests/agent/test_context_builder.py | 10 +- tests/agent/test_context_prompt_cache.py | 4 +- .../agent/test_document_extraction_toggle.py | 12 +- tests/agent/test_dream.py | 8 +- tests/agent/test_loop_save_turn.py | 81 +++++---- tests/agent/test_loop_tool_context.py | 2 +- tests/agent/test_mcp_transient_retry.py | 2 +- tests/agent/test_tool_loader_entrypoints.py | 2 +- tests/agent/tools/test_subagent_tools.py | 2 +- tests/tools/test_exec_session_tools.py | 2 +- tests/tools/test_mcp_tool.py | 16 +- tests/tools/test_tool_validation.py | 10 +- tests/tools/test_web_search_tool.py | 4 +- 26 files changed, 144 insertions(+), 236 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index 6ee1424b..1557a3e7 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -69,7 +69,7 @@ class ContextBuilder: def build_system_prompt( self, - skill_names: list[str] | None = None, + *, channel: str | None = None, session_summary: str | None = None, workspace: Path | None = None, @@ -196,14 +196,11 @@ class ContextBuilder: self, history: list[dict[str, Any]], current_message: str, - skill_names: list[str] | None = None, + *, media: list[str] | None = None, channel: str | None = None, - chat_id: str | None = None, current_role: str = "user", - sender_id: str | None = None, session_summary: str | None = None, - session_metadata: Mapping[str, Any] | None = None, runtime_context_blocks: Sequence[RuntimeContextBlock] | None = None, workspace: Path | None = None, include_memory_recent_history: bool = True, @@ -219,7 +216,6 @@ class ContextBuilder: { "role": "system", "content": self.build_system_prompt( - skill_names, channel=channel, session_summary=session_summary, workspace=root, diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 84b1f8d7..5d81bee0 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -13,7 +13,7 @@ from dataclasses import dataclass, field from enum import Enum, auto from functools import partial from pathlib import Path -from typing import TYPE_CHECKING, Any, Awaitable, Callable +from typing import TYPE_CHECKING, Any, Awaitable, Callable, TypeVar from loguru import logger @@ -103,15 +103,7 @@ if TYPE_CHECKING: ) from nanobot.cron.service import CronService -class TurnState(Enum): - RESTORE = auto() - COMPACT = auto() - COMMAND = auto() - BUILD = auto() - RUN = auto() - SAVE = auto() - RESPOND = auto() - DONE = auto() +_T = TypeVar("_T") class TurnKind(Enum): @@ -119,20 +111,10 @@ class TurnKind(Enum): SYSTEM = auto() -@dataclass -class StateTraceEntry: - state: TurnState - started_at: float - duration_ms: float - event: str - error: str | None = None - - @dataclass class TurnContext: msg: InboundMessage session_key: str - state: TurnState turn_id: str runtime: LLMRuntime | None kind: TurnKind @@ -146,7 +128,6 @@ class TurnContext: runtime_context_blocks: list[RuntimeContextBlock] = field(default_factory=list) final_content: str | None = None - tools_used: list[str] = field(default_factory=list) all_messages: list[dict[str, Any]] = field(default_factory=list) stop_reason: str = "" had_injections: bool = False @@ -178,8 +159,6 @@ class TurnContext: visible_run_started_at: float | None = None turn_latency_ms: int | None = None - trace: list[StateTraceEntry] = field(default_factory=list) - class AgentLoop: """ @@ -244,19 +223,6 @@ class AgentLoop: _RUNTIME_CHECKPOINT_KEY = "runtime_checkpoint" _PENDING_USER_TURN_KEY = "pending_user_turn" - # Event-driven state transition table. - # Handlers return an event string; the driver looks up the next state here. - _TRANSITIONS: dict[tuple[TurnState, str], TurnState] = { - (TurnState.RESTORE, "ok"): TurnState.COMPACT, - (TurnState.COMPACT, "ok"): TurnState.COMMAND, - (TurnState.COMMAND, "dispatch"): TurnState.BUILD, - (TurnState.COMMAND, "shortcut"): TurnState.DONE, - (TurnState.BUILD, "ok"): TurnState.RUN, - (TurnState.RUN, "ok"): TurnState.SAVE, - (TurnState.SAVE, "ok"): TurnState.RESPOND, - (TurnState.RESPOND, "ok"): TurnState.DONE, - } - def __init__( self, bus: MessageBus, @@ -712,13 +678,8 @@ class AgentLoop: current_message=ctx.msg.content, media=ctx.msg.media if ctx.kind is TurnKind.USER and ctx.msg.media else None, channel=ctx.delivery.route.channel, - chat_id=str( - ctx.msg.metadata.get("context_chat_id") or ctx.delivery.route.chat_id - ), current_role="user", - sender_id=ctx.msg.sender_id, session_summary=ctx.pending_summary, - session_metadata=ctx.session.metadata, workspace=scope.project_path, runtime_context_blocks=ctx.runtime_context_blocks, include_memory_recent_history=not ctx.ephemeral, @@ -1379,7 +1340,6 @@ class AgentLoop: msg=msg, session=None, session_key=key, - state=TurnState.RESTORE, turn_id=f"{key}:{time.time_ns()}", runtime=runtime, kind=kind, @@ -1449,65 +1409,47 @@ class AgentLoop: ctx.on_stream = _tracked_stream ctx.on_stream_end = _tracked_stream_end - while ctx.state is not TurnState.DONE: - handler_name = f"_state_{ctx.state.name.lower()}" - handler = getattr(self, handler_name, None) - if handler is None: - raise RuntimeError(f"Missing state handler for {ctx.state}") - - t0 = time.perf_counter() - try: - event = await handler(ctx) - except Exception: - duration = (time.perf_counter() - t0) * 1000 - ctx.trace.append( - StateTraceEntry( - state=ctx.state, - started_at=t0, - duration_ms=duration, - event="", - error="exception", - ) - ) - raise - - duration = (time.perf_counter() - t0) * 1000 - ctx.trace.append( - StateTraceEntry( - state=ctx.state, - started_at=t0, - duration_ms=duration, - event=event, - ) - ) - logger.debug( - "[turn {}] State {} took {:.1f}ms -> event {}", - ctx.turn_id, - ctx.state.name, - duration, - event, - ) - - next_state = self._TRANSITIONS.get((ctx.state, event)) - if next_state is None: - raise RuntimeError( - f"[turn {ctx.turn_id}] No transition from {ctx.state} " - f"on event {event!r}" - ) - ctx.state = next_state - - logger.debug( - "[turn {}] Turn completed after {} states", - ctx.turn_id, - len(ctx.trace), - ) + await self._run_turn_stage(ctx, "restore", self._restore_turn) + await self._run_turn_stage(ctx, "compact", self._compact_session) + if await self._run_turn_stage(ctx, "command", self._dispatch_command): + return ctx.outbound + await self._run_turn_stage(ctx, "build", self._build_turn) + await self._run_turn_stage(ctx, "run", self._run_turn) + await self._run_turn_stage(ctx, "save", self._persist_turn) + await self._run_turn_stage(ctx, "respond", self._prepare_outbound) return ctx.outbound + async def _run_turn_stage( + self, + ctx: TurnContext, + name: str, + handler: Callable[[TurnContext], Awaitable[_T]], + ) -> _T: + started_at = time.perf_counter() + try: + result = await handler(ctx) + except Exception: + duration_ms = (time.perf_counter() - started_at) * 1000 + logger.debug( + "[turn {}] Stage {} failed after {:.1f}ms", + ctx.turn_id, + name, + duration_ms, + ) + raise + duration_ms = (time.perf_counter() - started_at) * 1000 + logger.debug( + "[turn {}] Stage {} completed in {:.1f}ms", + ctx.turn_id, + name, + duration_ms, + ) + return result + def _assemble_outbound( self, msg: InboundMessage, final_content: str, - all_msgs: list[dict[str, Any]], stop_reason: str, had_injections: bool, streamed_content: bool, @@ -1538,7 +1480,7 @@ class AgentLoop: metadata=meta, ) - async def _state_restore(self, ctx: TurnContext) -> TurnState: + async def _restore_turn(self, ctx: TurnContext) -> None: """Restore checkpoint / pending user turn; extract documents.""" msg = ctx.msg @@ -1571,8 +1513,6 @@ class AgentLoop: if self._restore_pending_user_turn(ctx.session): self.sessions.save(ctx.session) - return "ok" - def _prepare_message_media(self, content: str, media: list[str]) -> tuple[str, list[str]]: if self._should_extract_document_text(): return extract_documents(content, media) @@ -1583,14 +1523,13 @@ class AgentLoop: return True return self.channels_config.extract_document_text - async def _state_compact(self, ctx: TurnContext) -> str: + async def _compact_session(self, ctx: TurnContext) -> None: ctx.session, pending = self.auto_compact.prepare_session(ctx.session, ctx.session_key) ctx.pending_summary = pending - return "ok" - async def _state_command(self, ctx: TurnContext) -> str: + async def _dispatch_command(self, ctx: TurnContext) -> bool: if ctx.kind is TurnKind.SYSTEM: - return "dispatch" + return False raw = ctx.msg.content.strip() _, automation_metadata = automation_history_overrides(ctx.msg.metadata) is_user_turn = ( @@ -1626,10 +1565,10 @@ class AgentLoop: ) self.sessions.save(ctx.session) self._clear_pending_user_turn(ctx.session) - return "shortcut" - return "dispatch" + return True + return False - async def _state_build(self, ctx: TurnContext) -> str: + async def _build_turn(self, ctx: TurnContext) -> None: runtime = ctx.runtime if runtime is None: runtime = self.runtime_for_session(ctx.session) @@ -1685,9 +1624,7 @@ class AgentLoop: if ctx.on_retry_wait is None: ctx.on_retry_wait = ctx.delivery.retry_wait_callback() - return "ok" - - async def _state_run(self, ctx: TurnContext) -> str: + async def _run_turn(self, ctx: TurnContext) -> None: if ctx.visible_run_started_at is None: ctx.visible_run_started_at = time.time() await ctx.delivery.running(started_at=ctx.visible_run_started_at) @@ -1714,17 +1651,15 @@ class AgentLoop: tools=ctx.tools, request_context=ctx.request_context, ) - final_content, tools_used, all_msgs, stop_reason, had_injections = result + final_content, _, all_msgs, stop_reason, had_injections = result ctx.final_content = final_content - ctx.tools_used = tools_used ctx.all_messages = all_msgs ctx.stop_reason = stop_reason ctx.had_injections = had_injections if ctx.kind is TurnKind.USER: await turn_continuation.maybe_continue_turn(ctx) - return "ok" - async def _state_save(self, ctx: TurnContext) -> str: + async def _persist_turn(self, ctx: TurnContext) -> None: turn_continuation.prepare_save_boundary(ctx) if ( @@ -1765,12 +1700,11 @@ class AgentLoop: self._clear_pending_user_turn(ctx.session) self._clear_runtime_checkpoint(ctx.session) self.sessions.save(ctx.session) - return "ok" - async def _state_respond(self, ctx: TurnContext) -> str: + async def _prepare_outbound(self, ctx: TurnContext) -> None: if ctx.suppress_response: ctx.outbound = None - return "ok" + return if ctx.kind is TurnKind.SYSTEM: ctx.outbound = ctx.delivery.background_response( ctx.final_content, @@ -1778,11 +1712,10 @@ class AgentLoop: streamed=ctx.streamed_content, latency_ms=ctx.turn_latency_ms, ) - return "ok" + return ctx.outbound = self._assemble_outbound( ctx.msg, ctx.final_content, - ctx.all_messages, ctx.stop_reason, ctx.had_injections, ctx.streamed_content, @@ -1790,7 +1723,6 @@ class AgentLoop: ) if ctx.ephemeral and ctx.outbound is not None: ctx.outbound.metadata["_stop_reason"] = ctx.stop_reason - return "ok" def _sanitize_persisted_blocks( self, diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 5bc4bf3b..bf875282 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -912,7 +912,7 @@ class Consolidator: ) -> tuple[int, str]: """Estimate prompt size from the full unconsolidated session tail.""" history = self._full_unconsolidated_history(session) - channel, chat_id = (session.key.split(":", 1) if ":" in session.key else (None, None)) + channel = session.key.split(":", 1)[0] if ":" in session.key else None # Include archived summary in estimation so the budget accounts for it. meta = session.metadata.get("_last_summary") summary = meta.get("text") if isinstance(meta, dict) else (meta if isinstance(meta, str) else None) @@ -920,10 +920,7 @@ class Consolidator: history=history, current_message="[token-probe]", channel=channel, - chat_id=chat_id, - sender_id=None, session_summary=summary, - session_metadata=session.metadata, session_key=session.key, unified_session=self.unified_session, ) diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index 671208ce..3e381188 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -1327,7 +1327,7 @@ class AgentRunner: return payload, event, exc return payload, event, None - if is_tool_error_result(tool_call.name, result): + if is_tool_error_result(result): await hook.on_execute_tool_error(context, tool_call, tool, params, result) event = { "name": tool_call.name, diff --git a/nanobot/agent/tools/apply_patch.py b/nanobot/agent/tools/apply_patch.py index 8e94c596..43c526f5 100644 --- a/nanobot/agent/tools/apply_patch.py +++ b/nanobot/agent/tools/apply_patch.py @@ -39,12 +39,6 @@ def _validate_patch_path(path: str) -> str: return normalized -def _lines_to_text(lines: list[str]) -> str: - if not lines: - return "" - return "\n".join(lines) + "\n" - - def _text_line_count(text: str) -> int: if not text: return 0 diff --git a/nanobot/agent/tools/cron.py b/nanobot/agent/tools/cron.py index db30dfdd..89f389f1 100644 --- a/nanobot/agent/tools/cron.py +++ b/nanobot/agent/tools/cron.py @@ -28,7 +28,7 @@ _CRON_PARAMETERS = tool_parameters_schema( "(e.g., 'Send a reminder to WeChat: xxx' or 'Check system status and report'). " "Not used for action='list' or action='remove'." ), - every_seconds=IntegerSchema(0, description="Interval in seconds (for recurring tasks)"), + every_seconds=IntegerSchema(description="Interval in seconds (for recurring tasks)"), cron_expr=StringSchema("Cron expression like '0 9 * * *' (for scheduled tasks)"), tz=StringSchema( "Optional IANA timezone for cron expressions (e.g. 'America/Vancouver'). " @@ -138,8 +138,6 @@ class CronTool(Tool): tz: str | None = None, at: str | None = None, job_id: str | None = None, - deliver: bool = True, - **kwargs: Any, ) -> str: if action == "add": if self._in_cron_context.get(): diff --git a/nanobot/agent/tools/exec_session.py b/nanobot/agent/tools/exec_session.py index 1813a04b..9390bc02 100644 --- a/nanobot/agent/tools/exec_session.py +++ b/nanobot/agent/tools/exec_session.py @@ -447,7 +447,6 @@ def format_session_poll(session_id: str, poll: _SessionPoll) -> str: default=False, ), yield_time_ms=IntegerSchema( - DEFAULT_YIELD_MS, description="Milliseconds to wait before returning recent output (default 1000, max 30000).", minimum=0, maximum=MAX_YIELD_MS, @@ -458,20 +457,17 @@ def format_session_poll(session_id: str, poll: _SessionPoll) -> str: nullable=True, ), wait_timeout_ms=IntegerSchema( - DEFAULT_WAIT_FOR_MS, description="Maximum milliseconds to wait for wait_for text (default 10000, max 120000).", minimum=0, maximum=MAX_WAIT_FOR_MS, nullable=True, ), max_output_chars=IntegerSchema( - DEFAULT_MAX_OUTPUT_CHARS, description="Maximum output characters to return from this poll (default 10000, max 50000).", minimum=1000, maximum=MAX_OUTPUT_CHARS, ), max_output_tokens=IntegerSchema( - DEFAULT_MAX_OUTPUT_CHARS, description="Compatibility alias for max_output_chars. The current runtime uses a character budget.", minimum=1000, maximum=MAX_OUTPUT_CHARS, diff --git a/nanobot/agent/tools/filesystem.py b/nanobot/agent/tools/filesystem.py index f143e6d1..596dd133 100644 --- a/nanobot/agent/tools/filesystem.py +++ b/nanobot/agent/tools/filesystem.py @@ -226,12 +226,10 @@ def _builtin_skill_read_path(path: str) -> Path | None: tool_parameters_schema( path=StringSchema("The file path to read"), offset=IntegerSchema( - 1, description="Line number to start reading from (1-indexed, default 1)", minimum=1, ), limit=IntegerSchema( - 2000, description="Maximum number of lines to read (default 2000)", minimum=1, ), @@ -790,13 +788,11 @@ def _find_match(content: str, old_text: str) -> tuple[str | None, int]: new_text=StringSchema("The text to replace with"), replace_all=BooleanSchema(description="Replace all occurrences (default false)"), occurrence=IntegerSchema( - 1, description="Optional 1-based occurrence to replace when old_text appears multiple times.", minimum=1, nullable=True, ), line_hint=IntegerSchema( - 1, description=( "Optional exact 1-based target line copied from read_file. " "The selected old_text match must cover this line." @@ -805,7 +801,6 @@ def _find_match(content: str, old_text: str) -> tuple[str | None, int]: nullable=True, ), expected_replacements=IntegerSchema( - 1, description="Optional guard for the number of replacements that must be made.", minimum=1, nullable=True, @@ -1036,7 +1031,6 @@ class EditFileTool(_FsTool): path=StringSchema("The directory path to list"), recursive=BooleanSchema(description="Recursively list all files (default false)"), max_entries=IntegerSchema( - 200, description="Maximum entries to return (default 200)", minimum=1, ), diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 73d66b0d..47803283 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -1273,7 +1273,7 @@ async def reload_servers(state: Any, registry: ToolRegistry) -> dict[str, Any]: tools_removed = 0 for name in [*removed, *changed]: - tools_removed += _unregister_server_tools(state, registry, name) + tools_removed += _unregister_server_tools(registry, name) await _close_server(state, name) state._mcp_servers = next_servers @@ -1447,7 +1447,7 @@ async def _refresh_terminated_server( return current_tool logger.warning("MCP server '{}' session terminated; refreshing connection", server_name) - _unregister_server_tools(state, registry, server_name) + _unregister_server_tools(registry, server_name) await _close_server(state, server_name) connected = await connect_mcp_servers({server_name: cfg}, registry) @@ -1479,7 +1479,7 @@ def _tool_belongs_to_server(tool: Tool | None, tool_name: str, server_name: str) return tool_name.startswith(_tool_prefix(server_name)) -def _unregister_server_tools(state: Any, registry: ToolRegistry, server_name: str) -> int: +def _unregister_server_tools(registry: ToolRegistry, server_name: str) -> int: removed = 0 for tool_name in list(registry.tool_names): tool = registry.get(tool_name) diff --git a/nanobot/agent/tools/registry.py b/nanobot/agent/tools/registry.py index eb0b4f4f..e2122284 100644 --- a/nanobot/agent/tools/registry.py +++ b/nanobot/agent/tools/registry.py @@ -12,7 +12,7 @@ if TYPE_CHECKING: from nanobot.runtime_context import RuntimeContextProvider -def is_tool_error_result(name: str, result: Any) -> bool: +def is_tool_error_result(result: Any) -> bool: return isinstance(result, ToolResult) and result.is_error @@ -193,7 +193,7 @@ class ToolRegistry: try: assert tool is not None # guarded by prepare_call() result = await tool.execute(**params) - if is_tool_error_result(name, result): + if is_tool_error_result(result): return ToolResult.error(str(result) + hint) return result except Exception as e: diff --git a/nanobot/agent/tools/schema.py b/nanobot/agent/tools/schema.py index e590368a..6ce11024 100644 --- a/nanobot/agent/tools/schema.py +++ b/nanobot/agent/tools/schema.py @@ -52,11 +52,10 @@ class StringSchema(Schema): class IntegerSchema(Schema): - """Integer parameter: optional placeholder int (legacy ctor signature), description, and bounds.""" + """Integer parameter with a description and optional bounds.""" def __init__( self, - value: int = 0, *, description: str = "", minimum: int | None = None, @@ -64,7 +63,6 @@ class IntegerSchema(Schema): enum: tuple[int, ...] | list[int] | None = None, nullable: bool = False, ) -> None: - self._value = value self._description = description self._minimum = minimum self._maximum = maximum @@ -92,7 +90,6 @@ class NumberSchema(Schema): def __init__( self, - value: float = 0.0, *, description: str = "", minimum: float | None = None, @@ -100,7 +97,6 @@ class NumberSchema(Schema): enum: tuple[float, ...] | list[float] | None = None, nullable: bool = False, ) -> None: - self._value = value self._description = description self._minimum = minimum self._maximum = maximum diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 71b369c8..6650a2af 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -108,7 +108,6 @@ class _PreparedCommand: working_dir=StringSchema("Optional working directory for the command"), workdir=StringSchema("Compatibility alias for working_dir"), timeout=IntegerSchema( - 60, description=( "Timeout in seconds. Increase for long-running commands " "like compilation or installation (default 60, max 600)." diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index bcae4bbd..834988cf 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -271,13 +271,12 @@ def _normalize_volcengine_auth_level(value: Any) -> int | None: @tool_parameters( tool_parameters_schema( query=StringSchema("Search query"), - count=IntegerSchema(1, description="Results (1-10)", minimum=1, maximum=10), + count=IntegerSchema(description="Results (1-10)", minimum=1, maximum=10), timeRange=StringSchema( "Optional time filter for providers that support it: " "OneDay, OneWeek, OneMonth, OneYear, or YYYY-MM-DD..YYYY-MM-DD", ), authLevel=IntegerSchema( - 0, description="Optional authority filter for providers that support it: 0=all, 1=authoritative", minimum=0, maximum=1, @@ -939,7 +938,7 @@ class WebSearchTool(Tool): "enum": ["markdown", "text"], "default": "markdown", }, - maxChars=IntegerSchema(0, minimum=100), + maxChars=IntegerSchema(minimum=100), required=["url"], ) ) diff --git a/tests/agent/test_context_builder.py b/tests/agent/test_context_builder.py index 70ccf74d..e4c22723 100644 --- a/tests/agent/test_context_builder.py +++ b/tests/agent/test_context_builder.py @@ -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" diff --git a/tests/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index 4c0aae25..1eefae67 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -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", ) diff --git a/tests/agent/test_document_extraction_toggle.py b/tests/agent/test_document_extraction_toggle.py index 29f8d6c1..a7536fdd 100644 --- a/tests/agent/test_document_extraction_toggle.py +++ b/tests/agent/test_document_extraction_toggle.py @@ -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 diff --git a/tests/agent/test_dream.py b/tests/agent/test_dream.py index 998d0ae3..f34ad17a 100644 --- a/tests/agent/test_dream.py +++ b/tests/agent/test_dream.py @@ -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 diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index 59946f91..9cef1932 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -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"] diff --git a/tests/agent/test_loop_tool_context.py b/tests/agent/test_loop_tool_context.py index 83cfcd10..2b6c1596 100644 --- a/tests/agent/test_loop_tool_context.py +++ b/tests/agent/test_loop_tool_context.py @@ -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( diff --git a/tests/agent/test_mcp_transient_retry.py b/tests/agent/test_mcp_transient_retry.py index a76b1f1a..bd9b4e72 100644 --- a/tests/agent/test_mcp_transient_retry.py +++ b/tests/agent/test_mcp_transient_retry.py @@ -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 diff --git a/tests/agent/test_tool_loader_entrypoints.py b/tests/agent/test_tool_loader_entrypoints.py index b898aaab..ace6924d 100644 --- a/tests/agent/test_tool_loader_entrypoints.py +++ b/tests/agent/test_tool_loader_entrypoints.py @@ -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" diff --git a/tests/agent/tools/test_subagent_tools.py b/tests/agent/tools/test_subagent_tools.py index ec496325..82555e65 100644 --- a/tests/agent/tools/test_subagent_tools.py +++ b/tests/agent/tools/test_subagent_tools.py @@ -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 == {} diff --git a/tests/tools/test_exec_session_tools.py b/tests/tools/test_exec_session_tools.py index b0fe2fee..fc45d31c 100644 --- a/tests/tools/test_exec_session_tools.py +++ b/tests/tools/test_exec_session_tools.py @@ -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): diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 05f57ea4..229be014 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -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 diff --git a/tests/tools/test_tool_validation.py b/tests/tools/test_tool_validation.py index 8ffd83e4..fdb881f4 100644 --- a/tests/tools/test_tool_validation.py +++ b/tests/tools/test_tool_validation.py @@ -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(""), diff --git a/tests/tools/test_web_search_tool.py b/tests/tools/test_web_search_tool.py index b7ad7d60..c9130b5d 100644 --- a/tests/tools/test_web_search_tool.py +++ b/tests/tools/test_web_search_tool.py @@ -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