From ef9e687f19ee4b8fe203e4f2b77cc015eccb1d9a Mon Sep 17 00:00:00 2001 From: chengyongru <61816729+chengyongru@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:07:58 +0800 Subject: [PATCH] refactor(core): remove redundant runtime scaffolding (#5127) --- nanobot/agent/context.py | 6 +- nanobot/agent/context_governance.py | 10 +-- nanobot/agent/loop.py | 36 +++------ nanobot/agent/tools/message.py | 26 +------ nanobot/bus/runtime_events.py | 16 ---- nanobot/providers/factory.py | 22 +++--- nanobot/providers/fallback_provider.py | 25 ++----- nanobot/session/webui_turns.py | 41 +--------- tests/agent/test_context_builder.py | 19 +++++ tests/agent/test_context_prompt_cache.py | 27 +++---- tests/agent/test_loop_save_turn.py | 57 +------------- tests/agent/test_runner_governance.py | 11 ++- tests/agent/test_task_cancel.py | 4 +- tests/agent/test_unified_session.py | 8 +- tests/cli/test_restart_command.py | 2 +- .../test_enforce_role_alternation.py | 9 +-- tests/test_openai_api.py | 2 + tests/tools/test_exec_session_tools.py | 6 +- tests/tools/test_message_tool.py | 74 ------------------- 19 files changed, 89 insertions(+), 312 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index 1557a3e76..61abf32cc 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -87,9 +87,9 @@ class ContextBuilder: parts.append(render_template("agent/tool_contract.md")) - memory = self.memory.get_memory_context() - if memory and not self._is_template_content(self.memory.read_memory(), "memory/MEMORY.md"): - parts.append(f"# Memory\n\n{memory}") + memory = self.memory.read_memory() + if memory and not self._is_template_content(memory, "memory/MEMORY.md"): + parts.append(f"# Memory\n\n## Long-term Memory\n{memory}") always_skills = self.skills.get_always_skills() if always_skills: diff --git a/nanobot/agent/context_governance.py b/nanobot/agent/context_governance.py index 838982b28..9a1a18776 100644 --- a/nanobot/agent/context_governance.py +++ b/nanobot/agent/context_governance.py @@ -26,7 +26,6 @@ if TYPE_CHECKING: from nanobot.providers.base import LLMProvider SNIP_SAFETY_BUFFER = 1024 -MICROCOMPACT_KEEP_RECENT = 10 MICROCOMPACT_MIN_CHARS = 500 INFLIGHT_COMPACT_TARGET_RATIO = 0.85 COMPACTABLE_TOOLS = frozenset({ @@ -498,14 +497,7 @@ class ContextGovernor: continue compactable.append((idx, str(tool_call_id))) - if not compactable: - return [] - primary_count = max(0, len(compactable) - MICROCOMPACT_KEEP_RECENT) - primary = compactable[:primary_count] - # Hard overflow beats the keep-recent preference. Return recent results - # after stale ones so the newest result is naturally last. - fallback = compactable[primary_count:] - return primary + fallback + return compactable def _compact_tool_result_at(self, messages: list[dict[str, Any]], idx: int) -> None: messages[idx]["content"] = self._tool_result_compaction_message(messages[idx]) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index b3a3dad97..7e8e4a9ad 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -43,11 +43,7 @@ from nanobot.agent.turn_hooks import AgentTurnHookSpec, build_agent_turn_hook from nanobot.bus.events import InboundMessage, OutboundMessage from nanobot.bus.outbound_events import StreamedResponseEvent from nanobot.bus.queue import MessageBus -from nanobot.bus.runtime_events import ( - RuntimeEventBus, - RuntimeEventPublisher, - ensure_runtime_event_publisher, -) +from nanobot.bus.runtime_events import RuntimeEventBus from nanobot.command import CommandContext, CommandRouter, register_builtin_commands from nanobot.config.schema import AgentDefaults, ModelPresetConfig from nanobot.providers.base import LLMProvider @@ -378,8 +374,8 @@ class AgentLoop: self._mcp_stacks: dict[str, MCPConnection] = {} self._mcp_connecting = False self._runtime_context_providers: list[RuntimeContextProvider] = [] - self._active_tasks: dict[str, list[asyncio.Task]] = {} # session_key -> tasks - self._background_tasks: list[asyncio.Task] = [] + self._active_tasks: dict[str, set[asyncio.Task[Any]]] = {} + self._background_tasks: set[asyncio.Task[Any]] = set() self._session_locks: dict[str, asyncio.Lock] = {} # Per-session pending queues for mid-turn message injection. # When a session has an active task, new messages for that session @@ -544,7 +540,7 @@ class AgentLoop: return if self._runtime_model_publisher is not None: self._runtime_model_publisher(runtime.model, runtime.model_preset) - self._runtime_events().runtime_model_changed( + self.runtime_event_publisher.runtime_model_changed( runtime.model, runtime.model_preset, ) @@ -621,9 +617,6 @@ class AgentLoop: if provider not in self._runtime_context_providers: self._runtime_context_providers.append(provider) - def _runtime_events(self) -> RuntimeEventPublisher: - return ensure_runtime_event_publisher(self) - async def submit_cron_turn(self, msg: InboundMessage) -> OutboundMessage | None: return await self._cron_turns.submit(msg) @@ -687,7 +680,6 @@ 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, - current_role="user", session_summary=ctx.pending_summary, workspace=scope.project_path, runtime_context_blocks=ctx.runtime_context_blocks, @@ -759,7 +751,7 @@ class AgentLoop: Returns the total number of cancelled tasks + subagents. """ - tasks = self._active_tasks.pop(key, []) + tasks = self._active_tasks.pop(key, set()) cancelled = sum(1 for t in tasks if not t.done() and t.cancel()) for t in tasks: with suppress(asyncio.CancelledError, Exception): @@ -1153,13 +1145,9 @@ class AgentLoop: # Compute the effective session key before dispatching # This ensures /stop command can find tasks correctly when unified session is enabled task = asyncio.create_task(self._dispatch(msg)) - self._active_tasks.setdefault(effective_key, []).append(task) - task.add_done_callback( - lambda t, k=effective_key: self._active_tasks.get(k, []) - and self._active_tasks[k].remove(t) - if t in self._active_tasks.get(k, []) - else None - ) + active_tasks = self._active_tasks.setdefault(effective_key, set()) + active_tasks.add(task) + task.add_done_callback(active_tasks.discard) finally: # MCP stdio transports use AnyIO cancel scopes; close them from the task that opened them. await self.close_mcp() @@ -1302,8 +1290,8 @@ class AgentLoop: def _schedule_background(self, coro) -> None: """Schedule a coroutine as a tracked background task (drained on shutdown).""" task = asyncio.create_task(coro) - self._background_tasks.append(task) - task.add_done_callback(self._background_tasks.remove) + self._background_tasks.add(task) + task.add_done_callback(self._background_tasks.discard) def stop(self) -> None: """Stop the agent loop.""" @@ -2036,5 +2024,5 @@ class AgentLoop: **kwargs, ) finally: - await self._runtime_events().run_status_changed(msg, session_key, "idle") - self._runtime_events().clear_turn(session_key) + await self.runtime_event_publisher.run_status_changed(msg, session_key, "idle") + self.runtime_event_publisher.clear_turn(session_key) diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index dad393d56..d8a660090 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -67,14 +67,6 @@ class MessageTool(Tool): self._fallback_message_id = default_message_id self._fallback_metadata: dict[str, Any] = {} self._sent_in_turn_var: ContextVar[bool] = ContextVar("message_sent_in_turn", default=False) - self._turn_delivered_media_var: ContextVar[tuple[str, ...]] = ContextVar( - "message_turn_delivered_media", - default=(), - ) - self._record_channel_delivery_var: ContextVar[bool] = ContextVar( - "message_record_channel_delivery", - default=False, - ) self._suppress_delivery_var: ContextVar[bool] = ContextVar( "message_suppress_delivery", default=False, @@ -96,19 +88,6 @@ class MessageTool(Tool): def start_turn(self) -> None: """Reset per-turn send tracking.""" self._sent_in_turn = False - self._turn_delivered_media_var.set(()) - - def turn_delivered_media_paths(self) -> list[str]: - """Absolute paths attached via this tool to the active chat in the current turn.""" - return list(self._turn_delivered_media_var.get()) - - def set_record_channel_delivery(self, active: bool): - """Mark tool-sent messages as proactive channel deliveries.""" - return self._record_channel_delivery_var.set(active) - - def reset_record_channel_delivery(self, token) -> None: - """Restore previous proactive delivery recording state.""" - self._record_channel_delivery_var.reset(token) def set_suppress_delivery(self, active: bool): """Acknowledge but don't deliver tool sends (heartbeat internal check).""" @@ -241,7 +220,7 @@ class MessageTool(Tool): metadata = dict(default_metadata) if same_target else {} if message_id: metadata["message_id"] = message_id - if self._record_channel_delivery_var.get() or media: + if media: metadata["_record_channel_delivery"] = True msg = OutboundMessage( @@ -261,9 +240,6 @@ class MessageTool(Tool): await self._send_callback(msg) if channel == default_channel and chat_id == default_chat_id: self._sent_in_turn = True - if media: - prev = self._turn_delivered_media_var.get() - self._turn_delivered_media_var.set(prev + tuple(str(p) for p in media)) media_info = f" with {len(media)} attachments" if media else "" button_info = f" with {sum(len(row) for row in buttons)} button(s)" if buttons else "" return f"Message sent to {channel}:{chat_id}{media_info}{button_info}" diff --git a/nanobot/bus/runtime_events.py b/nanobot/bus/runtime_events.py index fabe3c9b9..599aa12e0 100644 --- a/nanobot/bus/runtime_events.py +++ b/nanobot/bus/runtime_events.py @@ -233,19 +233,3 @@ class RuntimeEventPublisher: self.bus.publish_nowait( RuntimeModelChanged(model=model, model_preset=model_preset) ) - - -def ensure_runtime_event_publisher(owner: Any) -> RuntimeEventPublisher: - """Return an owner's runtime publisher, creating missing state lazily.""" - publisher = getattr(owner, "runtime_event_publisher", None) - if isinstance(publisher, RuntimeEventPublisher): - return publisher - - bus = getattr(owner, "runtime_events", None) - if not isinstance(bus, RuntimeEventBus): - bus = RuntimeEventBus() - owner.runtime_events = bus - - publisher = RuntimeEventPublisher(bus) - owner.runtime_event_publisher = publisher - return publisher diff --git a/nanobot/providers/factory.py b/nanobot/providers/factory.py index 54971560d..d57e13a11 100644 --- a/nanobot/providers/factory.py +++ b/nanobot/providers/factory.py @@ -43,15 +43,13 @@ def _provider_extra_headers( def _make_provider_core( config: Config, *, - preset_name: str | None = None, - preset: ModelPresetConfig | None = None, + preset: ModelPresetConfig, model: str | None = None, ) -> LLMProvider: """Create a plain LLM provider without failover wrapping.""" - resolved = _resolve_model_preset(config, preset_name=preset_name, preset=preset) - model = model or resolved.model - provider_name = config.get_provider_name(model, preset=resolved) - p = config.get_provider(model, preset=resolved) + model = model or preset.model + provider_name = config.get_provider_name(model, preset=preset) + p = config.get_provider(model, preset=preset) spec = find_by_name(provider_name) if provider_name else None if provider_name and not spec and p: if not p.api_base: @@ -120,7 +118,7 @@ def _make_provider_core( provider = AnthropicProvider( api_key=p.api_key if p else None, - api_base=config.get_api_base(model, preset=resolved), + api_base=config.get_api_base(model, preset=preset), default_model=model, extra_headers=_provider_extra_headers(spec, p), ) @@ -140,7 +138,7 @@ def _make_provider_core( provider = OpenAICompatProvider( api_key=p.api_key if p else None, - api_base=config.get_api_base(model, preset=resolved), + api_base=config.get_api_base(model, preset=preset), default_model=model, extra_headers=_provider_extra_headers(spec, p), spec=spec, @@ -150,7 +148,7 @@ def _make_provider_core( proxy=p.proxy if p else None, ) - provider.generation = resolved.to_generation_settings() + provider.generation = preset.to_generation_settings() return provider @@ -197,16 +195,14 @@ def make_provider( the failover path to create providers for fallback models. """ resolved = _resolve_model_preset(config, preset_name=preset_name, preset=preset) - provider = _make_provider_core(config, preset_name=preset_name, preset=preset, model=model) + provider = _make_provider_core(config, preset=resolved, model=model) fallback_presets = _resolve_fallback_presets(config, resolved) if fallback_presets: provider = FallbackProvider( primary=provider, fallback_presets=fallback_presets, - provider_factory=lambda fb: _make_provider_core( - config, preset_name=preset_name, preset=fb - ), + provider_factory=lambda fb: _make_provider_core(config, preset=fb), ) return provider diff --git a/nanobot/providers/fallback_provider.py b/nanobot/providers/fallback_provider.py index 93f2550f6..8b19890a5 100644 --- a/nanobot/providers/fallback_provider.py +++ b/nanobot/providers/fallback_provider.py @@ -13,7 +13,6 @@ from nanobot.providers.base import LLMProvider, LLMResponse # Circuit breaker tuned to match OpenAICompatProvider's Responses API breaker. _PRIMARY_FAILURE_THRESHOLD = 3 _PRIMARY_COOLDOWN_S = 60 -_MISSING = object() _FALLBACK_ERROR_KINDS = frozenset({ "timeout", "connection", @@ -279,25 +278,17 @@ class FallbackProvider(LLMProvider): await self._notify_fallback_model(fallback_model) - original_values = { - name: kwargs.get(name, _MISSING) - for name in ("model", "max_tokens", "temperature", "reasoning_effort") + fallback_kwargs = { + **kwargs, + "model": fallback_model, + "max_tokens": fallback.max_tokens, + "temperature": fallback.temperature, } - kwargs["model"] = fallback_model - kwargs["max_tokens"] = fallback.max_tokens - kwargs["temperature"] = fallback.temperature if fallback.reasoning_effort is None: - kwargs.pop("reasoning_effort", None) + fallback_kwargs.pop("reasoning_effort", None) else: - kwargs["reasoning_effort"] = fallback.reasoning_effort - try: - fallback_response = await call(fallback_provider, kwargs) - finally: - for name, value in original_values.items(): - if value is _MISSING: - kwargs.pop(name, None) - else: - kwargs[name] = value + fallback_kwargs["reasoning_effort"] = fallback.reasoning_effort + fallback_response = await call(fallback_provider, fallback_kwargs) if fallback_response.finish_reason != "error": logger.info( diff --git a/nanobot/session/webui_turns.py b/nanobot/session/webui_turns.py index d9ae091d8..38967ccce 100644 --- a/nanobot/session/webui_turns.py +++ b/nanobot/session/webui_turns.py @@ -5,7 +5,7 @@ from __future__ import annotations import re import time from collections.abc import Awaitable, Callable -from dataclasses import dataclass, field, replace +from dataclasses import dataclass, replace from typing import Any from uuid import uuid4 @@ -304,7 +304,6 @@ class WebuiTurnCoordinator: bus: MessageBus sessions: SessionManager schedule_background: Callable[[Awaitable[None]], None] - _title_contexts: dict[str, LLMRuntime] = field(default_factory=dict) def subscribe(self, runtime_events: RuntimeEventBus) -> Callable[[], None]: """Subscribe this coordinator to runtime events.""" @@ -408,18 +407,6 @@ class WebuiTurnCoordinator: ) ) - def capture_title_context( - self, - session_key: str, - msg: InboundMessage, - llm: LLMRuntime, - ) -> None: - if msg.channel == "websocket" and msg.metadata.get("webui") is True: - self._title_contexts[session_key] = llm - - def discard(self, session_key: str) -> None: - self._title_contexts.pop(session_key, None) - async def publish_run_status( self, msg: InboundMessage, @@ -451,32 +438,6 @@ class WebuiTurnCoordinator: metadata=msg.metadata, ) ) - self._schedule_title_update(msg, session_key=session_key) - - def _schedule_title_update(self, msg: InboundMessage, *, session_key: str) -> None: - title_context = self._title_contexts.pop(session_key, None) - if msg.metadata.get("webui") is not True or title_context is None: - return - - async def _generate_title_and_notify( - title_llm: LLMRuntime = title_context, - ) -> None: - generated = await maybe_generate_webui_title_after_turn( - channel=msg.channel, - metadata=msg.metadata, - sessions=self.sessions, - session_key=session_key, - provider=title_llm.provider, - model=title_llm.model, - ) - if generated: - await self._publish_session_metadata_updated( - channel=msg.channel, - chat_id=msg.chat_id, - metadata=msg.metadata, - ) - - self.schedule_background(_generate_title_and_notify()) def _schedule_title_update_from_event(self, event: TurnCompleted) -> None: title_context = event.runtime diff --git a/tests/agent/test_context_builder.py b/tests/agent/test_context_builder.py index e4c227235..b40f6f9ea 100644 --- a/tests/agent/test_context_builder.py +++ b/tests/agent/test_context_builder.py @@ -369,6 +369,25 @@ class TestBuildMessages: assert messages[1]["role"] == "user" assert "hello" in str(messages[1]["content"]) + def test_public_builder_preserves_assistant_role_compatibility(self, tmp_path): + from nanobot.agent import ContextBuilder as PublicContextBuilder + + builder = PublicContextBuilder(tmp_path) + messages = builder.build_messages( + history=[{"role": "assistant", "content": "previous result"}], + current_message="subagent result", + current_role="assistant", + runtime_context_blocks=[ + RuntimeContextBlock(source="test", content="user-only runtime context"), + ], + ) + + assert len(messages) == 2 + assert messages[-1]["role"] == "assistant" + assert messages[-1]["content"] == "previous result\n\nsubagent result" + assert "user-only runtime context" not in messages[-1]["content"] + assert "_meta" not in messages[-1] + def test_runtime_context_is_not_injected_by_default(self, tmp_path): builder = _builder(tmp_path) messages = builder.build_messages([], "hello", channel="cli") diff --git a/tests/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index 1eefae67c..daae5ab88 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -340,21 +340,6 @@ def test_system_prompt_keeps_message_tool_out_of_current_chat_replies(tmp_path) assert "Wait for the tool results, then answer once" in prompt -def test_subagent_result_does_not_create_consecutive_assistant_messages(tmp_path) -> None: - workspace = _make_workspace(tmp_path) - builder = ContextBuilder(workspace) - - messages = builder.build_messages( - history=[{"role": "assistant", "content": "previous result"}], - current_message="subagent result", - channel="cli", - current_role="assistant", - ) - - for left, right in zip(messages, messages[1:]): - assert not (left.get("role") == right.get("role") == "assistant") - - def test_memory_skill_is_lazy_loaded_from_skills_index(tmp_path) -> None: """Memory search guidance should be discoverable without loading its full body.""" workspace = _make_workspace(tmp_path) @@ -398,7 +383,7 @@ def test_template_memory_md_is_skipped(tmp_path) -> None: assert "This file is automatically updated by nanobot" not in prompt -def test_customized_memory_md_is_injected(tmp_path) -> None: +def test_customized_memory_md_is_injected(tmp_path, monkeypatch) -> None: """A Dream-populated MEMORY.md should be injected normally.""" workspace = _make_workspace(tmp_path) from nanobot.utils.helpers import sync_workspace_templates @@ -409,7 +394,17 @@ def test_customized_memory_md_is_injected(tmp_path) -> None: ) builder = ContextBuilder(workspace) + read_memory = builder.memory.read_memory + calls = 0 + + def tracked_read_memory() -> str: + nonlocal calls + calls += 1 + return read_memory() + + monkeypatch.setattr(builder.memory, "read_memory", tracked_read_memory) prompt = builder.build_system_prompt() assert "# Memory\n\n## Long-term Memory" in prompt assert "User prefers dark mode" in prompt + assert calls == 1 diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index 9cef1932f..53627ddfd 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -34,7 +34,7 @@ from nanobot.session.keys import ( LAST_CHANNEL_METADATA_KEY, UNIFIED_SESSION_KEY, ) -from nanobot.session.manager import Session, SessionManager +from nanobot.session.manager import Session from nanobot.session.turn_continuation import ( INTERNAL_CONTINUATION_META, INTERNAL_CONTINUATION_RUN_STARTED_AT_META, @@ -49,7 +49,6 @@ from nanobot.session.webui_turns import ( maybe_generate_webui_title, ) from nanobot.triggers.local_session_turns import LOCAL_TRIGGER_META -from nanobot.utils.llm_runtime import LLMRuntime def _mk_loop() -> AgentLoop: @@ -314,55 +313,6 @@ async def test_generate_webui_title_ignores_cron_internal_turns(tmp_path: Path) loop.provider.chat_with_retry.assert_not_awaited() -def test_webui_title_update_uses_captured_llm_runtime( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - bus = MessageBus() - sessions = SessionManager(tmp_path) - scheduled: list[object] = [] - captured: dict[str, object] = {} - - async def fake_title_after_turn(**kwargs: object) -> bool: - captured.update(kwargs) - return False - - monkeypatch.setattr( - "nanobot.session.webui_turns.maybe_generate_webui_title_after_turn", - fake_title_after_turn, - ) - coordinator = WebuiTurnCoordinator( - bus=bus, - sessions=sessions, - schedule_background=lambda coro: scheduled.append(coro), - ) - provider = MagicMock() - msg = InboundMessage( - channel="websocket", - sender_id="u1", - chat_id="chat1", - content="say hello", - metadata={"webui": True}, - ) - - coordinator.capture_title_context( - "websocket:chat1", - msg, - LLMRuntime.capture(provider, "turn-model", context_window_tokens=32_768), - ) - asyncio.run(coordinator.handle_turn_end( - msg, - session_key="websocket:chat1", - latency_ms=None, - )) - - assert len(scheduled) == 1 - asyncio.run(scheduled[0]) # type: ignore[arg-type] - - assert captured["provider"] is provider - assert captured["model"] == "turn-model" - - def test_save_turn_keeps_multimodal_runtime_context_for_model_replay() -> None: loop = _mk_loop() session = Session(key="test:runtime-only") @@ -1386,7 +1336,7 @@ async def test_stop_preserves_runtime_checkpoint_for_next_turn(tmp_path: Path) - first_msg = InboundMessage(channel="feishu", sender_id="u1", chat_id="c4", content="keep progress") task = asyncio.create_task(loop._process_message(first_msg)) - loop._active_tasks[first_msg.session_key] = [task] + loop._active_tasks[first_msg.session_key] = {task} await asyncio.wait_for(checkpoint_saved.wait(), timeout=1.0) stop_msg = InboundMessage(channel="feishu", sender_id="u1", chat_id="c4", content="/stop") @@ -1451,7 +1401,7 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_ runtime = loop.llm_runtime() seen: dict[str, object] = {} - record_runtime = MagicMock(wraps=loop._runtime_events().record_turn_runtime) + record_runtime = MagicMock(wraps=loop.runtime_event_publisher.record_turn_runtime) loop.runtime_event_publisher.record_turn_runtime = record_runtime async def fake_run_agent_loop(initial_messages, **kwargs): @@ -1691,7 +1641,6 @@ def test_subagent_followup_uses_user_model_input_and_assistant_history(tmp_path: projected = builder.build_messages( history=history, current_message="subagent result", - current_role="user", channel="cli", ) diff --git a/tests/agent/test_runner_governance.py b/tests/agent/test_runner_governance.py index f4fe2965f..71a3c48cb 100644 --- a/tests/agent/test_runner_governance.py +++ b/tests/agent/test_runner_governance.py @@ -10,7 +10,6 @@ import pytest from agent.runner_helpers import make_run_spec from nanobot.agent.context_governance import ( BACKFILL_CONTENT, - MICROCOMPACT_KEEP_RECENT, ContextGovernanceConfig, ContextGovernor, ) @@ -495,7 +494,7 @@ def test_microcompact_skips_when_prompt_under_hard_budget(monkeypatch): tools = MagicMock() tools.get_definitions.return_value = [] - total = MICROCOMPACT_KEEP_RECENT + 5 + total = 15 long_content = "x" * 600 messages = _microcompact_messages(total=total, tool_name="read_file", content=long_content) spec = make_run_spec(provider, @@ -529,7 +528,7 @@ def test_microcompact_overflow_compacts_to_low_watermark(monkeypatch): tools = MagicMock() tools.get_definitions.return_value = [] - total = MICROCOMPACT_KEEP_RECENT + 8 + total = 18 long_content = "x" * 600 messages = _microcompact_messages(total=total, tool_name="read_file", content=long_content) spec = make_run_spec(provider, @@ -617,7 +616,7 @@ def test_context_governor_keeps_compaction_boundary_stable(monkeypatch): tools = MagicMock() tools.get_definitions.return_value = [] - total = MICROCOMPACT_KEEP_RECENT + 8 + total = 18 long_content = "x" * 600 messages = _microcompact_messages(total=total, tool_name="read_file", content=long_content) spec = make_run_spec(provider, @@ -658,7 +657,7 @@ def test_microcompact_preserves_short_results(monkeypatch): tools = MagicMock() tools.get_definitions.return_value = [] - total = MICROCOMPACT_KEEP_RECENT + 5 + total = 15 messages = _microcompact_messages(total=total, tool_name="exec", content="short") spec = make_run_spec(provider, initial_messages=messages, @@ -690,7 +689,7 @@ def test_microcompact_skips_non_compactable_tools(monkeypatch): tools = MagicMock() tools.get_definitions.return_value = [] - total = MICROCOMPACT_KEEP_RECENT + 5 + total = 15 long_content = "y" * 1000 messages = _microcompact_messages(total=total, tool_name="message", content=long_content) spec = make_run_spec(provider, diff --git a/tests/agent/test_task_cancel.py b/tests/agent/test_task_cancel.py index 7ec8d1bb3..8e51091b7 100644 --- a/tests/agent/test_task_cancel.py +++ b/tests/agent/test_task_cancel.py @@ -73,7 +73,7 @@ class TestHandleStop: task = asyncio.create_task(slow_task()) await asyncio.sleep(0) - loop._active_tasks["test:c1"] = [task] + loop._active_tasks["test:c1"] = {task} msg = InboundMessage(channel="test", sender_id="u1", chat_id="c1", content="/stop") ctx = CommandContext(msg=msg, session=None, key=msg.session_key, raw="/stop", loop=loop) @@ -100,7 +100,7 @@ class TestHandleStop: tasks = [asyncio.create_task(slow(i)) for i in range(2)] await asyncio.sleep(0) - loop._active_tasks["test:c1"] = tasks + loop._active_tasks["test:c1"] = set(tasks) msg = InboundMessage(channel="test", sender_id="u1", chat_id="c1", content="/stop") ctx = CommandContext(msg=msg, session=None, key=msg.session_key, raw="/stop", loop=loop) diff --git a/tests/agent/test_unified_session.py b/tests/agent/test_unified_session.py index febd4c265..456c5e79b 100644 --- a/tests/agent/test_unified_session.py +++ b/tests/agent/test_unified_session.py @@ -454,7 +454,7 @@ class TestStopCommandWithUnifiedSession: # Simulate the task creation flow (from _run loop) effective_key = UNIFIED_SESSION_KEY if loop._unified_session and not msg.session_key_override else msg.session_key task = asyncio.create_task(loop._dispatch(msg)) - loop._active_tasks.setdefault(effective_key, []).append(task) + loop._active_tasks.setdefault(effective_key, set()).add(task) # Wait for task to complete await task @@ -475,7 +475,7 @@ class TestStopCommandWithUnifiedSession: await asyncio.sleep(10) # Will be cancelled task = asyncio.create_task(long_running()) - loop._active_tasks[UNIFIED_SESSION_KEY] = [task] + loop._active_tasks[UNIFIED_SESSION_KEY] = {task} # Create a message that would have session_key=UNIFIED_SESSION_KEY after dispatch msg = InboundMessage( @@ -506,7 +506,7 @@ class TestStopCommandWithUnifiedSession: await asyncio.sleep(10) task = asyncio.create_task(long_running()) - loop._active_tasks[UNIFIED_SESSION_KEY] = [task] + loop._active_tasks[UNIFIED_SESSION_KEY] = {task} msg = InboundMessage( channel="telegram", chat_id="123456", @@ -533,7 +533,7 @@ class TestStopCommandWithUnifiedSession: task1 = asyncio.create_task(long_running()) task2 = asyncio.create_task(long_running()) - loop._active_tasks[UNIFIED_SESSION_KEY] = [task1, task2] + loop._active_tasks[UNIFIED_SESSION_KEY] = {task1, task2} # /stop from discord should cancel tasks started from telegram msg = InboundMessage( diff --git a/tests/cli/test_restart_command.py b/tests/cli/test_restart_command.py index bdd5cc08e..0409489fe 100644 --- a/tests/cli/test_restart_command.py +++ b/tests/cli/test_restart_command.py @@ -285,7 +285,7 @@ class TestRestartCommand: finished_task.done.return_value = True msg = InboundMessage(channel="telegram", sender_id="u1", chat_id="c1", content="/status") - loop._active_tasks[msg.session_key] = [running_task, finished_task] + loop._active_tasks[msg.session_key] = {running_task, finished_task} loop.subagents.get_running_count_by_session.return_value = 2 response = await loop._process_message(msg) diff --git a/tests/providers/test_enforce_role_alternation.py b/tests/providers/test_enforce_role_alternation.py index 335062ba8..332a5fcbe 100644 --- a/tests/providers/test_enforce_role_alternation.py +++ b/tests/providers/test_enforce_role_alternation.py @@ -132,12 +132,11 @@ class TestEnforceRoleAlternation: assert len(msgs) == 2 def test_trailing_assistant_recovered_as_user_when_only_system_remains(self): - """Subagent result injected as assistant message must not be silently dropped. + """A trailing assistant message must not be silently dropped. - When build_messages(current_role="assistant") produces [system, assistant], - _enforce_role_alternation would drop the assistant, leaving only [system]. - Most providers (e.g. Zhipu/GLM error 1214) reject such requests. - The trailing assistant should be recovered as a user message instead. + An externally supplied [system, assistant] sequence would otherwise leave + only [system]. Most providers reject such requests, so the trailing + assistant should be recovered as a user message instead. """ msgs = [ {"role": "system", "content": "You are helpful."}, diff --git a/tests/test_openai_api.py b/tests/test_openai_api.py index f557c19b0..a07729511 100644 --- a/tests/test_openai_api.py +++ b/tests/test_openai_api.py @@ -454,10 +454,12 @@ async def test_empty_response_falls_back_without_retry(aiohttp_client) -> None: async def test_process_direct_accepts_media() -> None: """process_direct should forward media paths to _process_message.""" from nanobot.agent.loop import AgentLoop + from nanobot.bus.runtime_events import RuntimeEventPublisher loop = AgentLoop.__new__(AgentLoop) loop._connect_mcp = AsyncMock() loop._session_locks = {} + loop.runtime_event_publisher = RuntimeEventPublisher() captured_msg = None diff --git a/tests/tools/test_exec_session_tools.py b/tests/tools/test_exec_session_tools.py index fc45d31c3..5300bb14f 100644 --- a/tests/tools/test_exec_session_tools.py +++ b/tests/tools/test_exec_session_tools.py @@ -584,7 +584,7 @@ def test_agent_loop_shutdown_closes_exec_sessions(tmp_path, monkeypatch): monkeypatch.setattr(agent_context, "close_mcp", lambda _state: asyncio.sleep(0)) loop = object.__new__(AgentLoop) - loop._background_tasks = [] + loop._background_tasks = set() loop._exec_session_manager = manager loop.subagents = SimpleNamespace(close=AsyncMock()) @@ -601,7 +601,7 @@ def test_agent_loop_shutdown_closes_exec_sessions(tmp_path, monkeypatch): def test_agent_loop_shutdown_attempts_all_cleanup_after_errors(monkeypatch): async def run() -> None: loop = object.__new__(AgentLoop) - loop._background_tasks = [] + loop._background_tasks = set() loop.subagents = SimpleNamespace( close=AsyncMock(side_effect=RuntimeError("subagent cleanup failed")), ) @@ -754,7 +754,7 @@ def test_terminate_by_owner_skips_sessions_without_owner_key(tmp_path): def test_agent_loop_shutdown_preserves_single_cleanup_error(monkeypatch): async def run() -> None: loop = object.__new__(AgentLoop) - loop._background_tasks = [] + loop._background_tasks = set() loop.subagents = SimpleNamespace( close=AsyncMock(side_effect=RuntimeError("subagent cleanup failed")), ) diff --git a/tests/tools/test_message_tool.py b/tests/tools/test_message_tool.py index 73f58dbab..67fe2bd06 100644 --- a/tests/tools/test_message_tool.py +++ b/tests/tools/test_message_tool.py @@ -61,26 +61,6 @@ async def test_message_tool_suppresses_delivery_when_active() -> None: assert sent[0].content == "real" -@pytest.mark.asyncio -async def test_message_tool_marks_channel_delivery_only_when_enabled() -> None: - sent: list[OutboundMessage] = [] - - async def _send(msg: OutboundMessage) -> None: - sent.append(msg) - - tool = MessageTool(send_callback=_send) - - await tool.execute(content="normal", channel="telegram", chat_id="1") - token = tool.set_record_channel_delivery(True) - try: - await tool.execute(content="cron", channel="telegram", chat_id="1") - finally: - tool.reset_record_channel_delivery(token) - - assert sent[0].metadata == {} - assert sent[1].metadata == {"_record_channel_delivery": True} - - @pytest.mark.asyncio async def test_message_tool_records_media_deliveries() -> None: sent: list[OutboundMessage] = [] @@ -330,60 +310,6 @@ async def test_message_tool_resolves_mixed_media_paths() -> None: ] -@pytest.mark.asyncio -async def test_message_tool_tracks_turn_media_for_same_target(tmp_path) -> None: - sent: list[OutboundMessage] = [] - - async def _send(msg: OutboundMessage) -> None: - sent.append(msg) - - tool = MessageTool(send_callback=_send) - f = tmp_path / "doc.md" - f.write_text("hello", encoding="utf-8") - with request_context(RequestContext(channel="websocket", chat_id="chat-1", metadata={})): - tool.start_turn() - await tool.execute( - content="see file", - channel="websocket", - chat_id="chat-1", - media=[str(f)], - ) - assert tool.turn_delivered_media_paths() == [str(f.resolve())] - - -@pytest.mark.asyncio -async def test_message_tool_start_turn_clears_tracked_media(tmp_path) -> None: - async def _send(msg: OutboundMessage) -> None: - pass - - tool = MessageTool(send_callback=_send) - f = tmp_path / "doc.md" - f.write_text("hello", encoding="utf-8") - with request_context(RequestContext(channel="websocket", chat_id="chat-1", metadata={})): - tool.start_turn() - await tool.execute(content="see file", media=[str(f)]) - tool.start_turn() - assert tool.turn_delivered_media_paths() == [] - - -@pytest.mark.asyncio -async def test_message_tool_cross_target_does_not_track_turn_media(tmp_path) -> None: - async def _send(msg: OutboundMessage) -> None: - pass - - tool = MessageTool(send_callback=_send) - f = tmp_path / "doc.md" - f.write_text("hello", encoding="utf-8") - with request_context(RequestContext(channel="websocket", chat_id="chat-1", metadata={})): - await tool.execute( - content="see file", - channel="telegram", - chat_id="tg-other", - media=[str(f)], - ) - assert tool.turn_delivered_media_paths() == [] - - @pytest.mark.asyncio async def test_message_tool_rejects_wrong_explicit_ws_chat_id(tmp_path) -> None: sent: list[OutboundMessage] = []