From dacc699293246ca3b3f94818136347254fc3202c Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 29 Jun 2026 10:53:33 +0800 Subject: [PATCH] fix(config): retire max messages setting --- nanobot/agent/loop.py | 9 +++--- nanobot/config/loader.py | 18 +++++++++++ nanobot/config/schema.py | 4 --- nanobot/session/manager.py | 5 ++-- tests/agent/test_max_messages_config.py | 40 ++++--------------------- tests/config/test_config_migration.py | 37 +++++++++++++++++++++++ 6 files changed, 69 insertions(+), 44 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 7ddb9bc0..7cf0e772 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -57,7 +57,7 @@ from nanobot.session.goal_state import ( sustained_goal_active, ) from nanobot.session.keys import UNIFIED_SESSION_KEY, session_key_for_channel -from nanobot.session.manager import Session, SessionManager +from nanobot.session.manager import DEFAULT_REPLAY_MAX_MESSAGES, Session, SessionManager from nanobot.utils.document import extract_documents, reference_non_image_attachments from nanobot.utils.helpers import image_placeholder_text from nanobot.utils.helpers import truncate_text as truncate_text_fn @@ -201,7 +201,7 @@ class AgentLoop: timezone: str | None = None, session_ttl_minutes: int = 0, consolidation_ratio: float = 0.5, - max_messages: int = 500, + max_messages: int = DEFAULT_REPLAY_MAX_MESSAGES, hooks: list[AgentHook] | None = None, unified_session: bool = False, disabled_skills: list[str] | None = None, @@ -292,7 +292,9 @@ class AgentLoop: llm_wall_timeout_for_session=lambda sk: runner_wall_llm_timeout_s(self.sessions, sk), ) self._unified_session = unified_session - self._max_messages = max_messages if max_messages > 0 else 500 + self._max_messages = ( + max_messages if max_messages > 0 else DEFAULT_REPLAY_MAX_MESSAGES + ) self._running = False self._mcp_servers = mcp_servers or {} self._mcp_stacks: dict[str, AsyncExitStack] = {} @@ -390,7 +392,6 @@ class AgentLoop: disabled_skills=defaults.disabled_skills, session_ttl_minutes=defaults.session_ttl_minutes, consolidation_ratio=defaults.consolidation_ratio, - max_messages=defaults.max_messages, tools_config=config.tools, model_presets=preset_helpers.configured_model_presets(config), model_preset=defaults.model_preset, diff --git a/nanobot/config/loader.py b/nanobot/config/loader.py index 0fd1aa4c..78436507 100644 --- a/nanobot/config/loader.py +++ b/nanobot/config/loader.py @@ -7,6 +7,7 @@ from pathlib import Path from typing import Any import pydantic +from loguru import logger from pydantic import BaseModel from nanobot.config.schema import Config, _resolve_tool_config_refs @@ -152,6 +153,23 @@ def _env_replace(match: re.Match[str]) -> str: def _migrate_config(data: dict) -> dict: """Migrate old config formats to current.""" + agents = data.get("agents", {}) + defaults = agents.get("defaults", {}) if isinstance(agents, dict) else {} + if isinstance(defaults, dict): + legacy_max_message_keys = [ + key for key in ("maxMessages", "max_messages") if key in defaults + ] + if legacy_max_message_keys: + for key in legacy_max_message_keys: + defaults.pop(key, None) + # TODO(next version): Remove this legacy cleanup branch; the schema + # will silently ignore this field once the warning grace period ends. + logger.warning( + "agents.defaults.maxMessages/max_messages is legacy and ignored; " + "replay max messages is now an internal safety cap. Remove it from " + "config. This compatibility warning will be removed in the next version." + ) + # Move tools.exec.restrictToWorkspace → tools.restrictToWorkspace tools = data.get("tools", {}) exec_cfg = tools.get("exec", {}) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index e6212323..d7da7d2c 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -154,10 +154,6 @@ class AgentDefaults(Base): validation_alias=AliasChoices("idleCompactAfterMinutes", "sessionTtlMinutes"), serialization_alias="idleCompactAfterMinutes", ) # Auto-compact idle threshold in minutes (0 = disabled) - max_messages: int = Field( - default=500, - ge=0, - ) # Last-resort max messages to replay from session history (0 = use default 500) consolidation_ratio: float = Field( default=0.5, ge=0.1, diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index 44e64c96..1370af5f 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -27,6 +27,7 @@ from nanobot.utils.helpers import ( from nanobot.utils.subagent_channel_display import scrub_subagent_announce_body FILE_MAX_MESSAGES = 2000 +DEFAULT_REPLAY_MAX_MESSAGES = 500 _MESSAGE_TIME_PREFIX_RE = re.compile(r"^\[Message Time: [^\]]+\]\n?") _LOCAL_IMAGE_BREADCRUMB_RE = re.compile(r"^\[image: (?:/|~)[^\]]+\]\s*$") _TOOL_CALL_ECHO_RE = re.compile(r'^\s*(?:generate_image|message)\([^)]*\)\s*$') @@ -132,7 +133,7 @@ class Session: def get_history( self, - max_messages: int = 500, + max_messages: int = DEFAULT_REPLAY_MAX_MESSAGES, *, max_tokens: int = 0, extend_to_user: bool = False, @@ -143,7 +144,7 @@ class Session: token budget from the tail (``max_tokens``) when provided. """ unconsolidated = self.messages[self.last_consolidated:] - max_messages = max_messages if max_messages > 0 else 500 + max_messages = max_messages if max_messages > 0 else DEFAULT_REPLAY_MAX_MESSAGES start_idx = recent_message_start_index( unconsolidated, max_messages, diff --git a/tests/agent/test_max_messages_config.py b/tests/agent/test_max_messages_config.py index ff2b48b9..2b0c111f 100644 --- a/tests/agent/test_max_messages_config.py +++ b/tests/agent/test_max_messages_config.py @@ -1,4 +1,4 @@ -"""Tests for max_messages config wiring into session history replay.""" +"""Tests for the internal max_messages replay cap.""" from __future__ import annotations @@ -11,9 +11,9 @@ from nanobot.agent.loop import AgentLoop from nanobot.bus.events import InboundMessage from nanobot.bus.queue import MessageBus from nanobot.providers.base import LLMResponse -from nanobot.session.manager import Session +from nanobot.session.manager import DEFAULT_REPLAY_MAX_MESSAGES, Session -DEFAULT_MAX_MESSAGES = 500 +DEFAULT_MAX_MESSAGES = DEFAULT_REPLAY_MAX_MESSAGES def _make_loop(tmp_path: Path, max_messages: int = DEFAULT_MAX_MESSAGES) -> AgentLoop: @@ -103,10 +103,10 @@ class TestGetHistoryWithMaxMessages: class TestMaxMessagesIntegration: - """Verify the config flows from AgentLoop into get_history calls.""" + """Verify AgentLoop passes the replay cap into get_history calls.""" @pytest.mark.asyncio - async def test_process_message_passes_config_to_history_call(self, tmp_path: Path) -> None: + async def test_process_message_passes_limit_to_history_call(self, tmp_path: Path) -> None: """The real message path should pass max_messages into session history replay.""" loop = _make_loop(tmp_path, max_messages=25) loop.provider.chat_with_retry = AsyncMock( @@ -127,7 +127,7 @@ class TestMaxMessagesIntegration: assert mock_hist.call_args.kwargs["extend_to_user"] is False @pytest.mark.asyncio - async def test_zero_config_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None: + async def test_zero_limit_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path, max_messages=0) loop.provider.chat_with_retry = AsyncMock( return_value=LLMResponse(content="ok", tool_calls=[], usage={}) @@ -182,31 +182,3 @@ class TestMaxMessagesIntegration: sent_text = "\n".join(str(message.get("content")) for message in sent_messages) assert "new question" in sent_text assert "long older turn" not in sent_text - - -class TestSchemaConfig: - """Verify the config schema accepts max_messages.""" - - def test_schema_default(self) -> None: - from nanobot.config.schema import AgentDefaults - - defaults = AgentDefaults() - assert defaults.max_messages == DEFAULT_MAX_MESSAGES - - def test_schema_accepts_zero_as_builtin_limit(self) -> None: - from nanobot.config.schema import AgentDefaults - - defaults = AgentDefaults(max_messages=0) - assert defaults.max_messages == 0 - - def test_schema_accepts_positive(self) -> None: - from nanobot.config.schema import AgentDefaults - - defaults = AgentDefaults(max_messages=25) - assert defaults.max_messages == 25 - - def test_schema_rejects_negative(self) -> None: - from nanobot.config.schema import AgentDefaults - - with pytest.raises(Exception): # Pydantic validation error - AgentDefaults(max_messages=-1) diff --git a/tests/config/test_config_migration.py b/tests/config/test_config_migration.py index 1183887f..0f7d408a 100644 --- a/tests/config/test_config_migration.py +++ b/tests/config/test_config_migration.py @@ -2,6 +2,8 @@ import json import socket from unittest.mock import patch +import pytest + from nanobot.config.loader import load_config, save_config from nanobot.security.network import validate_url_target @@ -93,6 +95,41 @@ def test_onboard_does_not_crash_with_legacy_memory_window(tmp_path, monkeypatch) assert result.exit_code == 0 +@pytest.mark.parametrize("field_name", ["maxMessages", "max_messages"]) +def test_load_config_warns_and_ignores_legacy_max_messages(tmp_path, field_name) -> None: + config_path = tmp_path / "config.json" + config_path.write_text( + json.dumps({"agents": {"defaults": {field_name: 25, "maxTokens": 1234}}}), + encoding="utf-8", + ) + + with patch("nanobot.config.loader.logger.warning") as warning: + config = load_config(config_path) + + assert config.agents.defaults.max_tokens == 1234 + assert not hasattr(config.agents.defaults, "max_messages") + warning.assert_called_once() + message = warning.call_args.args[0] + assert "legacy and ignored" in message + assert "next version" in message + + +def test_save_config_drops_legacy_max_messages(tmp_path) -> None: + config_path = tmp_path / "config.json" + config_path.write_text( + json.dumps({"agents": {"defaults": {"maxMessages": 25}}}), + encoding="utf-8", + ) + + with patch("nanobot.config.loader.logger.warning"): + config = load_config(config_path) + save_config(config, config_path) + saved = json.loads(config_path.read_text(encoding="utf-8")) + + assert "maxMessages" not in saved["agents"]["defaults"] + assert "max_messages" not in saved["agents"]["defaults"] + + def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None: from types import SimpleNamespace