From 3eeac4e8f89c979e9f922a7731100fa23a642c64 Mon Sep 17 00:00:00 2001 From: alairjt Date: Mon, 23 Feb 2026 13:59:49 -0300 Subject: [PATCH 1/5] Fix: handle non-string tool call arguments in memory consolidation Fixes #1042. When the LLM returns tool call arguments as a dict or JSON string instead of parsed values, memory consolidation would fail with "TypeError: data must be str, not dict". Changes: - Add type guard in MemoryStore.consolidate() to parse string arguments and reject unexpected types gracefully - Add regression tests covering dict args, string args, and edge cases --- nanobot/agent/memory.py | 7 ++ tests/test_memory_consolidation_types.py | 147 +++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 tests/test_memory_consolidation_types.py diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index cdbc49f..978b1bc 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -125,6 +125,13 @@ class MemoryStore: return False args = response.tool_calls[0].arguments + # Some providers return arguments as a JSON string instead of dict + if isinstance(args, str): + args = json.loads(args) + if not isinstance(args, dict): + logger.warning("Memory consolidation: unexpected arguments type %s", type(args).__name__) + return False + if entry := args.get("history_entry"): if not isinstance(entry, str): entry = json.dumps(entry, ensure_ascii=False) diff --git a/tests/test_memory_consolidation_types.py b/tests/test_memory_consolidation_types.py new file mode 100644 index 0000000..375c802 --- /dev/null +++ b/tests/test_memory_consolidation_types.py @@ -0,0 +1,147 @@ +"""Test MemoryStore.consolidate() handles non-string tool call arguments. + +Regression test for https://github.com/HKUDS/nanobot/issues/1042 +When memory consolidation receives dict values instead of strings from the LLM +tool call response, it should serialize them to JSON instead of raising TypeError. +""" + +import json +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from nanobot.agent.memory import MemoryStore +from nanobot.providers.base import LLMResponse, ToolCallRequest + + +def _make_session(message_count: int = 30, memory_window: int = 50): + """Create a mock session with messages.""" + session = MagicMock() + session.messages = [ + {"role": "user", "content": f"msg{i}", "timestamp": "2026-01-01 00:00"} + for i in range(message_count) + ] + session.last_consolidated = 0 + return session + + +def _make_tool_response(history_entry, memory_update): + """Create an LLMResponse with a save_memory tool call.""" + return LLMResponse( + content=None, + tool_calls=[ + ToolCallRequest( + id="call_1", + name="save_memory", + arguments={ + "history_entry": history_entry, + "memory_update": memory_update, + }, + ) + ], + ) + + +class TestMemoryConsolidationTypeHandling: + """Test that consolidation handles various argument types correctly.""" + + @pytest.mark.asyncio + async def test_string_arguments_work(self, tmp_path: Path) -> None: + """Normal case: LLM returns string arguments.""" + store = MemoryStore(tmp_path) + provider = AsyncMock() + provider.chat = AsyncMock( + return_value=_make_tool_response( + history_entry="[2026-01-01] User discussed testing.", + memory_update="# Memory\nUser likes testing.", + ) + ) + session = _make_session(message_count=60) + + result = await store.consolidate(session, provider, "test-model", memory_window=50) + + assert result is True + assert store.history_file.exists() + assert "[2026-01-01] User discussed testing." in store.history_file.read_text() + assert "User likes testing." in store.memory_file.read_text() + + @pytest.mark.asyncio + async def test_dict_arguments_serialized_to_json(self, tmp_path: Path) -> None: + """Issue #1042: LLM returns dict instead of string — must not raise TypeError.""" + store = MemoryStore(tmp_path) + provider = AsyncMock() + provider.chat = AsyncMock( + return_value=_make_tool_response( + history_entry={"timestamp": "2026-01-01", "summary": "User discussed testing."}, + memory_update={"facts": ["User likes testing"], "topics": ["testing"]}, + ) + ) + session = _make_session(message_count=60) + + result = await store.consolidate(session, provider, "test-model", memory_window=50) + + assert result is True + assert store.history_file.exists() + history_content = store.history_file.read_text() + parsed = json.loads(history_content.strip()) + assert parsed["summary"] == "User discussed testing." + + memory_content = store.memory_file.read_text() + parsed_mem = json.loads(memory_content) + assert "User likes testing" in parsed_mem["facts"] + + @pytest.mark.asyncio + async def test_string_arguments_as_raw_json(self, tmp_path: Path) -> None: + """Some providers return arguments as a JSON string instead of parsed dict.""" + store = MemoryStore(tmp_path) + provider = AsyncMock() + + # Simulate arguments being a JSON string (not yet parsed) + response = LLMResponse( + content=None, + tool_calls=[ + ToolCallRequest( + id="call_1", + name="save_memory", + arguments=json.dumps({ + "history_entry": "[2026-01-01] User discussed testing.", + "memory_update": "# Memory\nUser likes testing.", + }), + ) + ], + ) + provider.chat = AsyncMock(return_value=response) + session = _make_session(message_count=60) + + result = await store.consolidate(session, provider, "test-model", memory_window=50) + + assert result is True + assert "User discussed testing." in store.history_file.read_text() + + @pytest.mark.asyncio + async def test_no_tool_call_returns_false(self, tmp_path: Path) -> None: + """When LLM doesn't use the save_memory tool, return False.""" + store = MemoryStore(tmp_path) + provider = AsyncMock() + provider.chat = AsyncMock( + return_value=LLMResponse(content="I summarized the conversation.", tool_calls=[]) + ) + session = _make_session(message_count=60) + + result = await store.consolidate(session, provider, "test-model", memory_window=50) + + assert result is False + assert not store.history_file.exists() + + @pytest.mark.asyncio + async def test_skips_when_few_messages(self, tmp_path: Path) -> None: + """Consolidation should be a no-op when messages < keep_count.""" + store = MemoryStore(tmp_path) + provider = AsyncMock() + session = _make_session(message_count=10) + + result = await store.consolidate(session, provider, "test-model", memory_window=50) + + assert result is True + provider.chat.assert_not_called() From ef572259747a59625865ef7d7c6fc72edb448c0c Mon Sep 17 00:00:00 2001 From: coldxiangyu Date: Tue, 24 Feb 2026 18:19:47 +0800 Subject: [PATCH 2/5] fix(web): resolve API key on each call + improve error message - Defer Brave API key resolution to execute() time instead of __init__, so env var or config changes take effect without gateway restart - Improve error message to reference actual config path (tools.web.search.apiKey) instead of only mentioning env var Fixes #1069 (issues 1 and 2 of 3) --- nanobot/agent/tools/web.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index 90cdda8..4ca788c 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -58,12 +58,21 @@ class WebSearchTool(Tool): } def __init__(self, api_key: str | None = None, max_results: int = 5): - self.api_key = api_key or os.environ.get("BRAVE_API_KEY", "") + self._config_api_key = api_key self.max_results = max_results - + + def _resolve_api_key(self) -> str: + """Resolve API key on each call to support hot-reload and env var changes.""" + return self._config_api_key or os.environ.get("BRAVE_API_KEY", "") + async def execute(self, query: str, count: int | None = None, **kwargs: Any) -> str: - if not self.api_key: - return "Error: BRAVE_API_KEY not configured" + api_key = self._resolve_api_key() + if not api_key: + return ( + "Error: Brave Search API key not configured. " + "Set it in ~/.nanobot/config.json under tools.web.search.apiKey " + "(or export BRAVE_API_KEY), then restart the gateway." + ) try: n = min(max(count or self.max_results, 1), 10) @@ -71,7 +80,7 @@ class WebSearchTool(Tool): r = await client.get( "https://api.search.brave.com/res/v1/web/search", params={"q": query, "count": n}, - headers={"Accept": "application/json", "X-Subscription-Token": self.api_key}, + headers={"Accept": "application/json", "X-Subscription-Token": api_key}, timeout=10.0 ) r.raise_for_status() From ec55f7791256cfcec28d947296247aac72f5701b Mon Sep 17 00:00:00 2001 From: Re-bin Date: Tue, 24 Feb 2026 11:04:56 +0000 Subject: [PATCH 3/5] fix(heartbeat): replace HEARTBEAT_OK token with virtual tool-call decision --- nanobot/cli/commands.py | 17 ++-- nanobot/config/schema.py | 8 ++ nanobot/heartbeat/service.py | 162 +++++++++++++++++++++-------------- 3 files changed, 117 insertions(+), 70 deletions(-) diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index 90b9f44..ca71694 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -360,19 +360,19 @@ def gateway( return "cli", "direct" # Create heartbeat service - async def on_heartbeat(prompt: str) -> str: - """Execute heartbeat through the agent.""" + async def on_heartbeat_execute(tasks: str) -> str: + """Phase 2: execute heartbeat tasks through the full agent loop.""" channel, chat_id = _pick_heartbeat_target() async def _silent(*_args, **_kwargs): pass return await agent.process_direct( - prompt, + tasks, session_key="heartbeat", channel=channel, chat_id=chat_id, - on_progress=_silent, # suppress: heartbeat should not push progress to external channels + on_progress=_silent, ) async def on_heartbeat_notify(response: str) -> None: @@ -383,12 +383,15 @@ def gateway( return # No external channel available to deliver to await bus.publish_outbound(OutboundMessage(channel=channel, chat_id=chat_id, content=response)) + hb_cfg = config.gateway.heartbeat heartbeat = HeartbeatService( workspace=config.workspace_path, - on_heartbeat=on_heartbeat, + provider=provider, + model=agent.model, + on_execute=on_heartbeat_execute, on_notify=on_heartbeat_notify, - interval_s=30 * 60, # 30 minutes - enabled=True + interval_s=hb_cfg.interval_s, + enabled=hb_cfg.enabled, ) if channels.enabled_channels: diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index fe8dd83..215f38d 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -228,11 +228,19 @@ class ProvidersConfig(Base): github_copilot: ProviderConfig = Field(default_factory=ProviderConfig) # Github Copilot (OAuth) +class HeartbeatConfig(Base): + """Heartbeat service configuration.""" + + enabled: bool = True + interval_s: int = 30 * 60 # 30 minutes + + class GatewayConfig(Base): """Gateway/server configuration.""" host: str = "0.0.0.0" port: int = 18790 + heartbeat: HeartbeatConfig = Field(default_factory=HeartbeatConfig) class WebSearchConfig(Base): diff --git a/nanobot/heartbeat/service.py b/nanobot/heartbeat/service.py index cb1a1c7..e534017 100644 --- a/nanobot/heartbeat/service.py +++ b/nanobot/heartbeat/service.py @@ -1,80 +1,110 @@ """Heartbeat service - periodic agent wake-up to check for tasks.""" +from __future__ import annotations + import asyncio from pathlib import Path -from typing import Any, Callable, Coroutine +from typing import TYPE_CHECKING, Any, Callable, Coroutine from loguru import logger -# Default interval: 30 minutes -DEFAULT_HEARTBEAT_INTERVAL_S = 30 * 60 +if TYPE_CHECKING: + from nanobot.providers.base import LLMProvider -# Token the agent replies with when there is nothing to report -HEARTBEAT_OK_TOKEN = "HEARTBEAT_OK" - -# The prompt sent to agent during heartbeat -HEARTBEAT_PROMPT = ( - "Read HEARTBEAT.md in your workspace and follow any instructions listed there. " - f"If nothing needs attention, reply with exactly: {HEARTBEAT_OK_TOKEN}" -) - - -def _is_heartbeat_empty(content: str | None) -> bool: - """Check if HEARTBEAT.md has no actionable content.""" - if not content: - return True - - # Lines to skip: empty, headers, HTML comments, empty checkboxes - skip_patterns = {"- [ ]", "* [ ]", "- [x]", "* [x]"} - - for line in content.split("\n"): - line = line.strip() - if not line or line.startswith("#") or line.startswith("