From 3eeac4e8f89c979e9f922a7731100fa23a642c64 Mon Sep 17 00:00:00 2001 From: alairjt Date: Mon, 23 Feb 2026 13:59:49 -0300 Subject: [PATCH 1/9] 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 30361c9307f9014f49530d80abd5717bc97f554a Mon Sep 17 00:00:00 2001 From: Re-bin Date: Mon, 23 Feb 2026 18:28:09 +0000 Subject: [PATCH 2/9] refactor: replace cron usage docs in TOOLS.md with reference to cron skill --- nanobot/templates/TOOLS.md | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/nanobot/templates/TOOLS.md b/nanobot/templates/TOOLS.md index 757edd2..51c3a2d 100644 --- a/nanobot/templates/TOOLS.md +++ b/nanobot/templates/TOOLS.md @@ -10,27 +10,6 @@ This file documents non-obvious constraints and usage patterns. - Output is truncated at 10,000 characters - `restrictToWorkspace` config can limit file access to the workspace -## Cron — Scheduled Reminders +## cron — Scheduled Reminders -Use `exec` to create scheduled reminders: - -```bash -# Recurring: every day at 9am -nanobot cron add --name "morning" --message "Good morning!" --cron "0 9 * * *" - -# With timezone (--tz only works with --cron) -nanobot cron add --name "standup" --message "Standup time!" --cron "0 10 * * 1-5" --tz "Asia/Shanghai" - -# Recurring: every 2 hours -nanobot cron add --name "water" --message "Drink water!" --every 7200 - -# One-time: specific ISO time -nanobot cron add --name "meeting" --message "Meeting starts now!" --at "2025-01-31T15:00:00" - -# Deliver to a specific channel/user -nanobot cron add --name "reminder" --message "Check email" --at "2025-01-31T09:00:00" --deliver --to "USER_ID" --channel "CHANNEL" - -# Manage jobs -nanobot cron list -nanobot cron remove -``` +- Please refer to cron skill for usage. From 91e13d91ac1001d0e10fbe04fa13225c6efecb3a Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Tue, 24 Feb 2026 04:26:26 +0800 Subject: [PATCH 3/9] fix(email): allow proactive sends when autoReplyEnabled is false Previously, `autoReplyEnabled=false` would block ALL email sends, including proactive emails triggered from other channels (e.g., asking nanobot on Feishu to send an email). Now `autoReplyEnabled` only controls automatic replies to incoming emails, not proactive sends. This allows users to disable auto-replies while still being able to ask nanobot to send emails on demand. Changes: - Check if recipient is in `_last_subject_by_chat` to determine if it's a reply - Only skip sending when it's a reply AND auto_reply_enabled is false - Add test for proactive send with auto_reply_enabled=false - Update existing test to verify reply behavior --- nanobot/channels/email.py | 14 +++++---- tests/test_email_channel.py | 59 ++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/nanobot/channels/email.py b/nanobot/channels/email.py index 5dc05fb..16771fb 100644 --- a/nanobot/channels/email.py +++ b/nanobot/channels/email.py @@ -108,11 +108,6 @@ class EmailChannel(BaseChannel): logger.warning("Skip email send: consent_granted is false") return - force_send = bool((msg.metadata or {}).get("force_send")) - if not self.config.auto_reply_enabled and not force_send: - logger.info("Skip automatic email reply: auto_reply_enabled is false") - return - if not self.config.smtp_host: logger.warning("Email channel SMTP host not configured") return @@ -122,6 +117,15 @@ class EmailChannel(BaseChannel): logger.warning("Email channel missing recipient address") return + # Determine if this is a reply (recipient has sent us an email before) + is_reply = to_addr in self._last_subject_by_chat + force_send = bool((msg.metadata or {}).get("force_send")) + + # autoReplyEnabled only controls automatic replies, not proactive sends + if is_reply and not self.config.auto_reply_enabled and not force_send: + logger.info("Skip automatic email reply to {}: auto_reply_enabled is false", to_addr) + return + base_subject = self._last_subject_by_chat.get(to_addr, "nanobot reply") subject = self._reply_subject(base_subject) if msg.metadata and isinstance(msg.metadata.get("subject"), str): diff --git a/tests/test_email_channel.py b/tests/test_email_channel.py index 8b22d8d..adf35a8 100644 --- a/tests/test_email_channel.py +++ b/tests/test_email_channel.py @@ -169,7 +169,8 @@ async def test_send_uses_smtp_and_reply_subject(monkeypatch) -> None: @pytest.mark.asyncio -async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None: +async def test_send_skips_reply_when_auto_reply_disabled(monkeypatch) -> None: + """When auto_reply_enabled=False, replies should be skipped but proactive sends allowed.""" class FakeSMTP: def __init__(self, _host: str, _port: int, timeout: int = 30) -> None: self.sent_messages: list[EmailMessage] = [] @@ -201,6 +202,11 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None: cfg = _make_config() cfg.auto_reply_enabled = False channel = EmailChannel(cfg, MessageBus()) + + # Mark alice as someone who sent us an email (making this a "reply") + channel._last_subject_by_chat["alice@example.com"] = "Previous email" + + # Reply should be skipped (auto_reply_enabled=False) await channel.send( OutboundMessage( channel="email", @@ -210,6 +216,7 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None: ) assert fake_instances == [] + # Reply with force_send=True should be sent await channel.send( OutboundMessage( channel="email", @@ -222,6 +229,56 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None: assert len(fake_instances[0].sent_messages) == 1 +@pytest.mark.asyncio +async def test_send_proactive_email_when_auto_reply_disabled(monkeypatch) -> None: + """Proactive emails (not replies) should be sent even when auto_reply_enabled=False.""" + class FakeSMTP: + def __init__(self, _host: str, _port: int, timeout: int = 30) -> None: + self.sent_messages: list[EmailMessage] = [] + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def starttls(self, context=None): + return None + + def login(self, _user: str, _pw: str): + return None + + def send_message(self, msg: EmailMessage): + self.sent_messages.append(msg) + + fake_instances: list[FakeSMTP] = [] + + def _smtp_factory(host: str, port: int, timeout: int = 30): + instance = FakeSMTP(host, port, timeout=timeout) + fake_instances.append(instance) + return instance + + monkeypatch.setattr("nanobot.channels.email.smtplib.SMTP", _smtp_factory) + + cfg = _make_config() + cfg.auto_reply_enabled = False + channel = EmailChannel(cfg, MessageBus()) + + # bob@example.com has never sent us an email (proactive send) + # This should be sent even with auto_reply_enabled=False + await channel.send( + OutboundMessage( + channel="email", + chat_id="bob@example.com", + content="Hello, this is a proactive email.", + ) + ) + assert len(fake_instances) == 1 + assert len(fake_instances[0].sent_messages) == 1 + sent = fake_instances[0].sent_messages[0] + assert sent["To"] == "bob@example.com" + + @pytest.mark.asyncio async def test_send_skips_when_consent_not_granted(monkeypatch) -> None: class FakeSMTP: From 4f8033627e05ad3d92fa4e31a5fdfad4f3711273 Mon Sep 17 00:00:00 2001 From: "xzq.xu" Date: Tue, 24 Feb 2026 13:42:07 +0800 Subject: [PATCH 4/9] feat(feishu): support images in post (rich text) messages Co-authored-by: Cursor --- nanobot/channels/feishu.py | 54 ++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/nanobot/channels/feishu.py b/nanobot/channels/feishu.py index 2d50d74..480bf7b 100644 --- a/nanobot/channels/feishu.py +++ b/nanobot/channels/feishu.py @@ -180,21 +180,25 @@ def _extract_element_content(element: dict) -> list[str]: return parts -def _extract_post_text(content_json: dict) -> str: - """Extract plain text from Feishu post (rich text) message content. +def _extract_post_content(content_json: dict) -> tuple[str, list[str]]: + """Extract text and image keys from Feishu post (rich text) message content. Supports two formats: 1. Direct format: {"title": "...", "content": [...]} 2. Localized format: {"zh_cn": {"title": "...", "content": [...]}} + + Returns: + (text, image_keys) - extracted text and list of image keys """ - def extract_from_lang(lang_content: dict) -> str | None: + def extract_from_lang(lang_content: dict) -> tuple[str | None, list[str]]: if not isinstance(lang_content, dict): - return None + return None, [] title = lang_content.get("title", "") content_blocks = lang_content.get("content", []) if not isinstance(content_blocks, list): - return None + return None, [] text_parts = [] + image_keys = [] if title: text_parts.append(title) for block in content_blocks: @@ -209,22 +213,36 @@ def _extract_post_text(content_json: dict) -> str: text_parts.append(element.get("text", "")) elif tag == "at": text_parts.append(f"@{element.get('user_name', 'user')}") - return " ".join(text_parts).strip() if text_parts else None + elif tag == "img": + img_key = element.get("image_key") + if img_key: + image_keys.append(img_key) + text = " ".join(text_parts).strip() if text_parts else None + return text, image_keys # Try direct format first if "content" in content_json: - result = extract_from_lang(content_json) - if result: - return result + text, images = extract_from_lang(content_json) + if text or images: + return text or "", images # Try localized format for lang_key in ("zh_cn", "en_us", "ja_jp"): lang_content = content_json.get(lang_key) - result = extract_from_lang(lang_content) - if result: - return result + text, images = extract_from_lang(lang_content) + if text or images: + return text or "", images - return "" + return "", [] + + +def _extract_post_text(content_json: dict) -> str: + """Extract plain text from Feishu post (rich text) message content. + + Legacy wrapper for _extract_post_content, returns only text. + """ + text, _ = _extract_post_content(content_json) + return text class FeishuChannel(BaseChannel): @@ -691,9 +709,17 @@ class FeishuChannel(BaseChannel): content_parts.append(text) elif msg_type == "post": - text = _extract_post_text(content_json) + text, image_keys = _extract_post_content(content_json) if text: content_parts.append(text) + # Download images embedded in post + for img_key in image_keys: + file_path, content_text = await self._download_and_save_media( + "image", {"image_key": img_key}, message_id + ) + if file_path: + media_paths.append(file_path) + content_parts.append(content_text) elif msg_type in ("image", "audio", "file", "media"): file_path, content_text = await self._download_and_save_media(msg_type, content_json, message_id) From ef572259747a59625865ef7d7c6fc72edb448c0c Mon Sep 17 00:00:00 2001 From: coldxiangyu Date: Tue, 24 Feb 2026 18:19:47 +0800 Subject: [PATCH 5/9] 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 6/9] 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("