From 62bd54ac4a2433ecfaf10c478980d22c25a65cf0 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 13 Apr 2026 11:27:16 +0800 Subject: [PATCH 1/5] fix(agent): skip auto-compact for sessions with active agent tasks Prevent proactive compaction from archiving sessions that have an in-flight agent task, avoiding mid-turn context truncation when a task runs longer than the idle TTL. --- nanobot/agent/autocompact.py | 17 ++++-- nanobot/agent/loop.py | 5 +- tests/agent/test_auto_compact.py | 100 ++++++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 7 deletions(-) diff --git a/nanobot/agent/autocompact.py b/nanobot/agent/autocompact.py index 47c7b5a3..ce70337c 100644 --- a/nanobot/agent/autocompact.py +++ b/nanobot/agent/autocompact.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Collection from datetime import datetime from typing import TYPE_CHECKING, Any, Callable, Coroutine @@ -23,12 +24,13 @@ class AutoCompact: self._archiving: set[str] = set() self._summaries: dict[str, tuple[str, datetime]] = {} - def _is_expired(self, ts: datetime | str | None) -> bool: + def _is_expired(self, ts: datetime | str | None, + now: datetime | None = None) -> bool: if self._ttl <= 0 or not ts: return False if isinstance(ts, str): ts = datetime.fromisoformat(ts) - return (datetime.now() - ts).total_seconds() >= self._ttl * 60 + return ((now or datetime.now()) - ts).total_seconds() >= self._ttl * 60 @staticmethod def _format_summary(text: str, last_active: datetime) -> str: @@ -56,10 +58,17 @@ class AutoCompact: cut = len(tail) - len(kept) return tail[:cut], kept - def check_expired(self, schedule_background: Callable[[Coroutine], None]) -> None: + def check_expired(self, schedule_background: Callable[[Coroutine], None], + active_session_keys: Collection[str] = ()) -> None: + """Schedule archival for idle sessions, skipping those with in-flight agent tasks.""" + now = datetime.now() for info in self.sessions.list_sessions(): key = info.get("key", "") - if key and key not in self._archiving and self._is_expired(info.get("updated_at")): + if not key or key in self._archiving: + continue + if key in active_session_keys: + continue + if self._is_expired(info.get("updated_at"), now): self._archiving.add(key) logger.debug("Auto-compact: scheduling archival for {} (idle > {} min)", key, self._ttl) schedule_background(self._archive(key)) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 5631e12a..5c4fbfc4 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -433,7 +433,10 @@ class AgentLoop: try: msg = await asyncio.wait_for(self.bus.consume_inbound(), timeout=1.0) except asyncio.TimeoutError: - self.auto_compact.check_expired(self._schedule_background) + self.auto_compact.check_expired( + self._schedule_background, + active_session_keys=self._pending_queues.keys(), + ) continue except asyncio.CancelledError: # Preserve real task cancellation so shutdown can complete cleanly. diff --git a/tests/agent/test_auto_compact.py b/tests/agent/test_auto_compact.py index b3462820..1f6886ed 100644 --- a/tests/agent/test_auto_compact.py +++ b/tests/agent/test_auto_compact.py @@ -560,9 +560,12 @@ class TestProactiveAutoCompact: """Test proactive auto-new on idle ticks (TimeoutError path in run loop).""" @staticmethod - async def _run_check_expired(loop): + async def _run_check_expired(loop, active_session_keys=()): """Helper: run check_expired via callback and wait for background tasks.""" - loop.auto_compact.check_expired(loop._schedule_background) + loop.auto_compact.check_expired( + loop._schedule_background, + active_session_keys=active_session_keys, + ) await asyncio.sleep(0.1) @pytest.mark.asyncio @@ -701,6 +704,99 @@ class TestProactiveAutoCompact: assert not archive_called await loop.close_mcp() + @pytest.mark.asyncio + async def test_skip_expired_session_with_active_agent_task(self, tmp_path): + """Expired session with an active agent task should NOT be archived.""" + loop = _make_loop(tmp_path, session_ttl_minutes=15) + session = loop.sessions.get_or_create("cli:test") + _add_turns(session, 6, prefix="old") + session.updated_at = datetime.now() - timedelta(minutes=20) + loop.sessions.save(session) + + archive_count = 0 + + async def _fake_archive(messages): + nonlocal archive_count + archive_count += 1 + return "Summary." + + loop.consolidator.archive = _fake_archive + + # Simulate an active agent task for this session + await self._run_check_expired(loop, active_session_keys={"cli:test"}) + assert archive_count == 0 + + session_after = loop.sessions.get_or_create("cli:test") + assert len(session_after.messages) == 12 # All messages preserved + + await loop.close_mcp() + + @pytest.mark.asyncio + async def test_archive_after_active_task_completes(self, tmp_path): + """Session should be archived on next tick after active task completes.""" + loop = _make_loop(tmp_path, session_ttl_minutes=15) + session = loop.sessions.get_or_create("cli:test") + _add_turns(session, 6, prefix="old") + session.updated_at = datetime.now() - timedelta(minutes=20) + loop.sessions.save(session) + + archive_count = 0 + + async def _fake_archive(messages): + nonlocal archive_count + archive_count += 1 + return "Summary." + + loop.consolidator.archive = _fake_archive + + # First tick: active task, skip + await self._run_check_expired(loop, active_session_keys={"cli:test"}) + assert archive_count == 0 + + # Second tick: task completed, should archive + await self._run_check_expired(loop) + assert archive_count == 1 + await loop.close_mcp() + + @pytest.mark.asyncio + async def test_partial_active_set_only_archives_inactive_expired(self, tmp_path): + """With multiple sessions, only the expired+inactive one should be archived.""" + loop = _make_loop(tmp_path, session_ttl_minutes=15) + # Session A: expired, no active task -> should be archived + s1 = loop.sessions.get_or_create("cli:expired_idle") + _add_turns(s1, 6, prefix="old_a") + s1.updated_at = datetime.now() - timedelta(minutes=20) + loop.sessions.save(s1) + # Session B: expired, has active task -> should be skipped + s2 = loop.sessions.get_or_create("cli:expired_active") + _add_turns(s2, 6, prefix="old_b") + s2.updated_at = datetime.now() - timedelta(minutes=20) + loop.sessions.save(s2) + # Session C: recent, no active task -> should be skipped + s3 = loop.sessions.get_or_create("cli:recent") + s3.add_message("user", "recent") + loop.sessions.save(s3) + + archive_count = 0 + + async def _fake_archive(messages): + nonlocal archive_count + archive_count += 1 + return "Summary." + + loop.consolidator.archive = _fake_archive + + await self._run_check_expired(loop, active_session_keys={"cli:expired_active"}) + + assert archive_count == 1 + s1_after = loop.sessions.get_or_create("cli:expired_idle") + assert len(s1_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + s2_after = loop.sessions.get_or_create("cli:expired_active") + assert len(s2_after.messages) == 12 # Preserved + s3_after = loop.sessions.get_or_create("cli:recent") + assert len(s3_after.messages) == 1 # Preserved + await loop.close_mcp() + @pytest.mark.asyncio async def test_no_reschedule_after_successful_archive(self, tmp_path): """Already-archived session should NOT be re-scheduled on subsequent ticks.""" From 89ea2375fdb7cffc98aab5b32e315ccc84fb5550 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 13 Apr 2026 11:30:54 +0800 Subject: [PATCH 2/5] fix(provider): recover trailing assistant message as user to prevent empty request When a subagent result is injected with current_role="assistant", _enforce_role_alternation drops the trailing assistant message, leaving only the system prompt. Providers like Zhipu/GLM reject such requests with error 1214 ("messages parameter invalid"). Now the last popped assistant message is recovered as a user message when no user/tool messages remain. --- nanobot/providers/base.py | 16 +++++++- .../test_enforce_role_alternation.py | 41 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/nanobot/providers/base.py b/nanobot/providers/base.py index 8ce2b9a7..759d880a 100644 --- a/nanobot/providers/base.py +++ b/nanobot/providers/base.py @@ -392,8 +392,22 @@ class LLMProvider(ABC): else: merged.append(dict(msg)) + last_popped = None while merged and merged[-1].get("role") == "assistant": - merged.pop() + last_popped = merged.pop() + + # If removing trailing assistant messages left only system messages, + # the request would be invalid for most providers (e.g. Zhipu/GLM + # error 1214). Recover by converting the last popped assistant + # message to a user message so the LLM can still see the content. + if ( + merged + and last_popped is not None + and not any(m.get("role") in ("user", "tool") for m in merged) + ): + recovered = dict(last_popped) + recovered["role"] = "user" + merged.append(recovered) return merged diff --git a/tests/providers/test_enforce_role_alternation.py b/tests/providers/test_enforce_role_alternation.py index aef57f47..333c5d04 100644 --- a/tests/providers/test_enforce_role_alternation.py +++ b/tests/providers/test_enforce_role_alternation.py @@ -131,6 +131,47 @@ class TestEnforceRoleAlternation: assert msgs[0] == original_first 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. + + 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. + """ + msgs = [ + {"role": "system", "content": "You are helpful."}, + {"role": "assistant", "content": "Subagent completed successfully."}, + ] + result = LLMProvider._enforce_role_alternation(msgs) + assert len(result) == 2 + assert result[0]["role"] == "system" + assert result[1]["role"] == "user" + assert "Subagent completed successfully." in result[1]["content"] + + def test_trailing_assistant_not_recovered_when_user_message_present(self): + """Recovery should NOT happen when a user message already exists.""" + msgs = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hi"}, + {"role": "assistant", "content": "Hello!"}, + ] + result = LLMProvider._enforce_role_alternation(msgs) + assert len(result) == 2 + assert result[-1]["role"] == "user" + + def test_trailing_assistant_recovered_with_tool_result_preceding(self): + """When only [system, tool, assistant] remains, recovery is not needed + because tool messages are valid non-system content.""" + msgs = [ + {"role": "system", "content": "You are helpful."}, + {"role": "tool", "content": "result", "tool_call_id": "1"}, + {"role": "assistant", "content": "Done."}, + ] + result = LLMProvider._enforce_role_alternation(msgs) + assert len(result) == 2 + assert result[-1]["role"] == "tool" + def test_only_assistant_messages(self): msgs = [ {"role": "assistant", "content": "A"}, From b311759e87fc56e8ff12f1bc2dcbdf648ee52723 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 13 Apr 2026 16:03:15 +0800 Subject: [PATCH 3/5] fix(log): remove noisy no-op logs from auto-compact Remove two debug log lines that fire on every idle channel check: - "scheduling archival" (logged before knowing if there's work) - "skipping, no un-consolidated messages" (the common no-op path) The meaningful "archived" info log (only on real work) is preserved. --- nanobot/agent/autocompact.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nanobot/agent/autocompact.py b/nanobot/agent/autocompact.py index ce70337c..9d9c1e29 100644 --- a/nanobot/agent/autocompact.py +++ b/nanobot/agent/autocompact.py @@ -70,7 +70,6 @@ class AutoCompact: continue if self._is_expired(info.get("updated_at"), now): self._archiving.add(key) - logger.debug("Auto-compact: scheduling archival for {} (idle > {} min)", key, self._ttl) schedule_background(self._archive(key)) async def _archive(self, key: str) -> None: @@ -79,7 +78,6 @@ class AutoCompact: session = self.sessions.get_or_create(key) archive_msgs, kept_msgs = self._split_unconsolidated(session) if not archive_msgs and not kept_msgs: - logger.debug("Auto-compact: skipping {}, no un-consolidated messages", key) session.updated_at = datetime.now() self.sessions.save(session) return From b3288fbc87e31a992d92f92e71bf6b1cd58fd24d Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 13 Apr 2026 16:49:01 +0800 Subject: [PATCH 4/5] fix(log): only log auto-compact when messages are actually archived --- nanobot/agent/autocompact.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/nanobot/agent/autocompact.py b/nanobot/agent/autocompact.py index 9d9c1e29..eabd8615 100644 --- a/nanobot/agent/autocompact.py +++ b/nanobot/agent/autocompact.py @@ -93,13 +93,14 @@ class AutoCompact: session.last_consolidated = 0 session.updated_at = datetime.now() self.sessions.save(session) - logger.info( - "Auto-compact: archived {} (archived={}, kept={}, summary={})", - key, - len(archive_msgs), - len(kept_msgs), - bool(summary), - ) + if archive_msgs: + logger.info( + "Auto-compact: archived {} (archived={}, kept={}, summary={})", + key, + len(archive_msgs), + len(kept_msgs), + bool(summary), + ) except Exception: logger.exception("Auto-compact: failed for {}", key) finally: From 0750d1f182a0a751f08b7d5c95cbd378da0d071c Mon Sep 17 00:00:00 2001 From: moranfong <274257964+zijiefang@users.noreply.github.com> Date: Mon, 13 Apr 2026 23:42:58 +0800 Subject: [PATCH 5/5] fix(config): return provider default api base in config resolution --- nanobot/config/schema.py | 6 ++---- tests/cli/test_commands.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index aa5ab993..fb891515 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -304,17 +304,15 @@ class Config(BaseSettings): return p.api_key if p else None def get_api_base(self, model: str | None = None) -> str | None: - """Get API base URL for the given model. Applies default URLs for gateway/local providers.""" + """Get API base URL for the given model, falling back to the provider default when present.""" from nanobot.providers.registry import find_by_name p, name = self._match_provider(model) if p and p.api_base: return p.api_base - # Only gateways get a default api_base here. Standard providers - # resolve their base URL from the registry in the provider constructor. if name: spec = find_by_name(name) - if spec and (spec.is_gateway or spec.is_local) and spec.default_api_base: + if spec and spec.default_api_base: return spec.default_api_base return None diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py index 3a1e7145..fd5429d8 100644 --- a/tests/cli/test_commands.py +++ b/tests/cli/test_commands.py @@ -264,6 +264,39 @@ def test_find_by_name_accepts_camel_case_and_hyphen_aliases(): assert find_by_name("github-copilot").name == "github_copilot" +def test_config_explicit_xiaomi_mimo_provider_uses_default_api_base(): + config = Config.model_validate( + { + "agents": { + "defaults": { + "provider": "xiaomi_mimo", + "model": "MiniMax-M1-80k", + } + }, + "providers": { + "xiaomiMimo": { + "apiKey": "test-key", + } + }, + } + ) + + assert config.get_provider_name() == "xiaomi_mimo" + assert config.get_api_base() == "https://api.xiaomimimo.com/v1" + + +def test_config_auto_detects_xiaomi_mimo_from_model_keyword(): + config = Config.model_validate( + { + "agents": {"defaults": {"provider": "auto", "model": "mimo/MiniMax-M1-80k"}}, + "providers": {"xiaomiMimo": {"apiKey": "test-key"}}, + } + ) + + assert config.get_provider_name() == "xiaomi_mimo" + assert config.get_api_base() == "https://api.xiaomimimo.com/v1" + + def test_config_auto_detects_ollama_from_local_api_base(): config = Config.model_validate( {