Merge remote-tracking branch 'origin/main' into pr-1961

Made-with: Cursor

# Conflicts:
#	.gitignore

tests/test_channel_plugins.py

@@ -0,0 +1,228 @@
+"""Tests for channel plugin discovery, merging, and config compatibility."""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+from unittest.mock import patch
+
+import pytest
+
+from nanobot.bus.events import OutboundMessage
+from nanobot.bus.queue import MessageBus
+from nanobot.channels.base import BaseChannel
+from nanobot.channels.manager import ChannelManager
+from nanobot.config.schema import ChannelsConfig
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+class _FakePlugin(BaseChannel):
+    name = "fakeplugin"
+    display_name = "Fake Plugin"
+
+    async def start(self) -> None:
+        pass
+
+    async def stop(self) -> None:
+        pass
+
+    async def send(self, msg: OutboundMessage) -> None:
+        pass
+
+
+class _FakeTelegram(BaseChannel):
+    """Plugin that tries to shadow built-in telegram."""
+    name = "telegram"
+    display_name = "Fake Telegram"
+
+    async def start(self) -> None:
+        pass
+
+    async def stop(self) -> None:
+        pass
+
+    async def send(self, msg: OutboundMessage) -> None:
+        pass
+
+
+def _make_entry_point(name: str, cls: type):
+    """Create a mock entry point that returns *cls* on load()."""
+    ep = SimpleNamespace(name=name, load=lambda _cls=cls: _cls)
+    return ep
+
+
+# ---------------------------------------------------------------------------
+# ChannelsConfig extra="allow"
+# ---------------------------------------------------------------------------
+
+def test_channels_config_accepts_unknown_keys():
+    cfg = ChannelsConfig.model_validate({
+        "myplugin": {"enabled": True, "token": "abc"},
+    })
+    extra = cfg.model_extra
+    assert extra is not None
+    assert extra["myplugin"]["enabled"] is True
+    assert extra["myplugin"]["token"] == "abc"
+
+
+def test_channels_config_getattr_returns_extra():
+    cfg = ChannelsConfig.model_validate({"myplugin": {"enabled": True}})
+    section = getattr(cfg, "myplugin", None)
+    assert isinstance(section, dict)
+    assert section["enabled"] is True
+
+
+def test_channels_config_builtin_fields_removed():
+    """After decoupling, ChannelsConfig has no explicit channel fields."""
+    cfg = ChannelsConfig()
+    assert not hasattr(cfg, "telegram")
+    assert cfg.send_progress is True
+    assert cfg.send_tool_hints is False
+
+
+# ---------------------------------------------------------------------------
+# discover_plugins
+# ---------------------------------------------------------------------------
+
+_EP_TARGET = "importlib.metadata.entry_points"
+
+
+def test_discover_plugins_loads_entry_points():
+    from nanobot.channels.registry import discover_plugins
+
+    ep = _make_entry_point("line", _FakePlugin)
+    with patch(_EP_TARGET, return_value=[ep]):
+        result = discover_plugins()
+
+    assert "line" in result
+    assert result["line"] is _FakePlugin
+
+
+def test_discover_plugins_handles_load_error():
+    from nanobot.channels.registry import discover_plugins
+
+    def _boom():
+        raise RuntimeError("broken")
+
+    ep = SimpleNamespace(name="broken", load=_boom)
+    with patch(_EP_TARGET, return_value=[ep]):
+        result = discover_plugins()
+
+    assert "broken" not in result
+
+
+# ---------------------------------------------------------------------------
+# discover_all — merge & priority
+# ---------------------------------------------------------------------------
+
+def test_discover_all_includes_builtins():
+    from nanobot.channels.registry import discover_all, discover_channel_names
+
+    with patch(_EP_TARGET, return_value=[]):
+        result = discover_all()
+
+    # discover_all() only returns channels that are actually available (dependencies installed)
+    # discover_channel_names() returns all built-in channel names
+    # So we check that all actually loaded channels are in the result
+    for name in result:
+        assert name in discover_channel_names()
+
+
+def test_discover_all_includes_external_plugin():
+    from nanobot.channels.registry import discover_all
+
+    ep = _make_entry_point("line", _FakePlugin)
+    with patch(_EP_TARGET, return_value=[ep]):
+        result = discover_all()
+
+    assert "line" in result
+    assert result["line"] is _FakePlugin
+
+
+def test_discover_all_builtin_shadows_plugin():
+    from nanobot.channels.registry import discover_all
+
+    ep = _make_entry_point("telegram", _FakeTelegram)
+    with patch(_EP_TARGET, return_value=[ep]):
+        result = discover_all()
+
+    assert "telegram" in result
+    assert result["telegram"] is not _FakeTelegram
+
+
+# ---------------------------------------------------------------------------
+# Manager _init_channels with dict config (plugin scenario)
+# ---------------------------------------------------------------------------
+
+@pytest.mark.asyncio
+async def test_manager_loads_plugin_from_dict_config():
+    """ChannelManager should instantiate a plugin channel from a raw dict config."""
+    from nanobot.channels.manager import ChannelManager
+
+    fake_config = SimpleNamespace(
+        channels=ChannelsConfig.model_validate({
+            "fakeplugin": {"enabled": True, "allowFrom": ["*"]},
+        }),
+        providers=SimpleNamespace(groq=SimpleNamespace(api_key="")),
+    )
+
+    with patch(
+        "nanobot.channels.registry.discover_all",
+        return_value={"fakeplugin": _FakePlugin},
+    ):
+        mgr = ChannelManager.__new__(ChannelManager)
+        mgr.config = fake_config
+        mgr.bus = MessageBus()
+        mgr.channels = {}
+        mgr._dispatch_task = None
+        mgr._init_channels()
+
+    assert "fakeplugin" in mgr.channels
+    assert isinstance(mgr.channels["fakeplugin"], _FakePlugin)
+
+
+@pytest.mark.asyncio
+async def test_manager_skips_disabled_plugin():
+    fake_config = SimpleNamespace(
+        channels=ChannelsConfig.model_validate({
+            "fakeplugin": {"enabled": False},
+        }),
+        providers=SimpleNamespace(groq=SimpleNamespace(api_key="")),
+    )
+
+    with patch(
+        "nanobot.channels.registry.discover_all",
+        return_value={"fakeplugin": _FakePlugin},
+    ):
+        mgr = ChannelManager.__new__(ChannelManager)
+        mgr.config = fake_config
+        mgr.bus = MessageBus()
+        mgr.channels = {}
+        mgr._dispatch_task = None
+        mgr._init_channels()
+
+    assert "fakeplugin" not in mgr.channels
+
+
+# ---------------------------------------------------------------------------
+# Built-in channel default_config() and dict->Pydantic conversion
+# ---------------------------------------------------------------------------
+
+def test_builtin_channel_default_config():
+    """Built-in channels expose default_config() returning a dict with 'enabled': False."""
+    from nanobot.channels.telegram import TelegramChannel
+    cfg = TelegramChannel.default_config()
+    assert isinstance(cfg, dict)
+    assert cfg["enabled"] is False
+    assert "token" in cfg
+
+
+def test_builtin_channel_init_from_dict():
+    """Built-in channels accept a raw dict and convert to Pydantic internally."""
+    from nanobot.channels.telegram import TelegramChannel
+    bus = MessageBus()
+    ch = TelegramChannel({"enabled": False, "token": "test-tok", "allowFrom": ["*"]}, bus)
+    assert ch.config.token == "test-tok"
+    assert ch.config.allow_from == ["*"]

tests/test_commands.py

@@ -1,3 +1,4 @@
+import re
 import shutil
 from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, patch
@@ -11,6 +12,12 @@ from nanobot.providers.litellm_provider import LiteLLMProvider
 from nanobot.providers.openai_codex_provider import _strip_model_prefix
 from nanobot.providers.registry import find_by_model
 
+
+def _strip_ansi(text):
+    """Remove ANSI escape codes from text."""
+    ansi_escape = re.compile(r'\x1b\[[0-9;]*m')
+    return ansi_escape.sub('', text)
+
 runner = CliRunner()
 
 
@@ -228,10 +235,11 @@ def test_agent_help_shows_workspace_and_config_options():
     result = runner.invoke(app, ["agent", "--help"])
 
     assert result.exit_code == 0
-    assert "--workspace" in result.stdout
-    assert "-w" in result.stdout
-    assert "--config" in result.stdout
-    assert "-c" in result.stdout
+    stripped_output = _strip_ansi(result.stdout)
+    assert "--workspace" in stripped_output
+    assert "-w" in stripped_output
+    assert "--config" in stripped_output
+    assert "-c" in stripped_output
 
 
 def test_agent_uses_default_config_when_no_workspace_or_config_flags(mock_agent_runtime):

tests/test_config_migration.py

@@ -1,4 +1,5 @@
 import json
+from types import SimpleNamespace
 
 from typer.testing import CliRunner
 
@@ -86,3 +87,46 @@ def test_onboard_refresh_rewrites_legacy_config_template(tmp_path, monkeypatch)
     assert defaults["maxTokens"] == 3333
     assert defaults["contextWindowTokens"] == 65_536
     assert "memoryWindow" not in defaults
+
+
+def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
+    config_path = tmp_path / "config.json"
+    workspace = tmp_path / "workspace"
+    config_path.write_text(
+        json.dumps(
+            {
+                "channels": {
+                    "qq": {
+                        "enabled": False,
+                        "appId": "",
+                        "secret": "",
+                        "allowFrom": [],
+                    }
+                }
+            }
+        ),
+        encoding="utf-8",
+    )
+
+    monkeypatch.setattr("nanobot.config.loader.get_config_path", lambda: config_path)
+    monkeypatch.setattr("nanobot.cli.commands.get_workspace_path", lambda: workspace)
+    monkeypatch.setattr(
+        "nanobot.channels.registry.discover_all",
+        lambda: {
+            "qq": SimpleNamespace(
+                default_config=lambda: {
+                    "enabled": False,
+                    "appId": "",
+                    "secret": "",
+                    "allowFrom": [],
+                    "msgFormat": "plain",
+                }
+            )
+        },
+    )
+
+    result = runner.invoke(app, ["onboard"], input="n\n")
+
+    assert result.exit_code == 0
+    saved = json.loads(config_path.read_text(encoding="utf-8"))
+    assert saved["channels"]["qq"]["msgFormat"] == "plain"

tests/test_consolidate_offset.py

@@ -505,7 +505,8 @@ class TestNewCommandArchival:
         return loop
 
     @pytest.mark.asyncio
-    async def test_new_does_not_clear_session_when_archive_fails(self, tmp_path: Path) -> None:
+    async def test_new_clears_session_immediately_even_if_archive_fails(self, tmp_path: Path) -> None:
+        """/new clears session immediately; archive_messages retries until raw dump."""
         from nanobot.bus.events import InboundMessage
 
         loop = self._make_loop(tmp_path)
@@ -514,9 +515,12 @@ class TestNewCommandArchival:
             session.add_message("user", f"msg{i}")
             session.add_message("assistant", f"resp{i}")
         loop.sessions.save(session)
-        before_count = len(session.messages)
+
+        call_count = 0
 
         async def _failing_consolidate(_messages) -> bool:
+            nonlocal call_count
+            call_count += 1
             return False
 
         loop.memory_consolidator.consolidate_messages = _failing_consolidate  # type: ignore[method-assign]
@@ -525,8 +529,13 @@ class TestNewCommandArchival:
         response = await loop._process_message(new_msg)
 
         assert response is not None
-        assert "failed" in response.content.lower()
-        assert len(loop.sessions.get_or_create("cli:test").messages) == before_count
+        assert "new session started" in response.content.lower()
+
+        session_after = loop.sessions.get_or_create("cli:test")
+        assert len(session_after.messages) == 0
+
+        await loop.close_mcp()
+        assert call_count == 3  # retried up to raw-archive threshold
 
     @pytest.mark.asyncio
     async def test_new_archives_only_unconsolidated_messages(self, tmp_path: Path) -> None:
@@ -554,6 +563,8 @@ class TestNewCommandArchival:
 
         assert response is not None
         assert "new session started" in response.content.lower()
+
+        await loop.close_mcp()
         assert archived_count == 3
 
     @pytest.mark.asyncio
@@ -578,3 +589,31 @@ class TestNewCommandArchival:
         assert response is not None
         assert "new session started" in response.content.lower()
         assert loop.sessions.get_or_create("cli:test").messages == []
+
+    @pytest.mark.asyncio
+    async def test_close_mcp_drains_pending_archives(self, tmp_path: Path) -> None:
+        """close_mcp waits for background archive tasks to complete."""
+        from nanobot.bus.events import InboundMessage
+
+        loop = self._make_loop(tmp_path)
+        session = loop.sessions.get_or_create("cli:test")
+        for i in range(3):
+            session.add_message("user", f"msg{i}")
+            session.add_message("assistant", f"resp{i}")
+        loop.sessions.save(session)
+
+        archived = asyncio.Event()
+
+        async def _slow_consolidate(_messages) -> bool:
+            await asyncio.sleep(0.1)
+            archived.set()
+            return True
+
+        loop.memory_consolidator.consolidate_messages = _slow_consolidate  # type: ignore[method-assign]
+
+        new_msg = InboundMessage(channel="cli", sender_id="user", chat_id="test", content="/new")
+        await loop._process_message(new_msg)
+
+        assert not archived.is_set()
+        await loop.close_mcp()
+        assert archived.is_set()

tests/test_dingtalk_channel.py

@@ -6,7 +6,7 @@ import pytest
 from nanobot.bus.queue import MessageBus
 import nanobot.channels.dingtalk as dingtalk_module
 from nanobot.channels.dingtalk import DingTalkChannel, NanobotDingTalkHandler
-from nanobot.config.schema import DingTalkConfig
+from nanobot.channels.dingtalk import DingTalkConfig
 
 
 class _FakeResponse:
@@ -14,19 +14,31 @@ class _FakeResponse:
         self.status_code = status_code
         self._json_body = json_body or {}
         self.text = "{}"
+        self.content = b""
+        self.headers = {"content-type": "application/json"}
 
     def json(self) -> dict:
         return self._json_body
 
 
 class _FakeHttp:
-    def __init__(self) -> None:
+    def __init__(self, responses: list[_FakeResponse] | None = None) -> None:
         self.calls: list[dict] = []
+        self._responses = list(responses) if responses else []
 
-    async def post(self, url: str, json=None, headers=None):
-        self.calls.append({"url": url, "json": json, "headers": headers})
+    def _next_response(self) -> _FakeResponse:
+        if self._responses:
+            return self._responses.pop(0)
         return _FakeResponse()
 
+    async def post(self, url: str, json=None, headers=None, **kwargs):
+        self.calls.append({"method": "POST", "url": url, "json": json, "headers": headers})
+        return self._next_response()
+
+    async def get(self, url: str, **kwargs):
+        self.calls.append({"method": "GET", "url": url})
+        return self._next_response()
+
 
 @pytest.mark.asyncio
 async def test_group_message_keeps_sender_id_and_routes_chat_id() -> None:
@@ -109,3 +121,93 @@ async def test_handler_uses_voice_recognition_text_when_text_is_empty(monkeypatc
     assert msg.content == "voice transcript"
     assert msg.sender_id == "user1"
     assert msg.chat_id == "group:conv123"
+
+
+@pytest.mark.asyncio
+async def test_handler_processes_file_message(monkeypatch) -> None:
+    """Test that file messages are handled and forwarded with downloaded path."""
+    bus = MessageBus()
+    channel = DingTalkChannel(
+        DingTalkConfig(client_id="app", client_secret="secret", allow_from=["user1"]),
+        bus,
+    )
+    handler = NanobotDingTalkHandler(channel)
+
+    class _FakeFileChatbotMessage:
+        text = None
+        extensions = {}
+        image_content = None
+        rich_text_content = None
+        sender_staff_id = "user1"
+        sender_id = "fallback-user"
+        sender_nick = "Alice"
+        message_type = "file"
+
+        @staticmethod
+        def from_dict(_data):
+            return _FakeFileChatbotMessage()
+
+    async def fake_download(download_code, filename, sender_id):
+        return f"/tmp/nanobot_dingtalk/{sender_id}/{filename}"
+
+    monkeypatch.setattr(dingtalk_module, "ChatbotMessage", _FakeFileChatbotMessage)
+    monkeypatch.setattr(dingtalk_module, "AckMessage", SimpleNamespace(STATUS_OK="OK"))
+    monkeypatch.setattr(channel, "_download_dingtalk_file", fake_download)
+
+    status, body = await handler.process(
+        SimpleNamespace(
+            data={
+                "conversationType": "1",
+                "content": {"downloadCode": "abc123", "fileName": "report.xlsx"},
+                "text": {"content": ""},
+            }
+        )
+    )
+
+    await asyncio.gather(*list(channel._background_tasks))
+    msg = await bus.consume_inbound()
+
+    assert (status, body) == ("OK", "OK")
+    assert "[File]" in msg.content
+    assert "/tmp/nanobot_dingtalk/user1/report.xlsx" in msg.content
+
+
+@pytest.mark.asyncio
+async def test_download_dingtalk_file(tmp_path, monkeypatch) -> None:
+    """Test the two-step file download flow (get URL then download content)."""
+    channel = DingTalkChannel(
+        DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
+        MessageBus(),
+    )
+
+    # Mock access token
+    async def fake_get_token():
+        return "test-token"
+
+    monkeypatch.setattr(channel, "_get_access_token", fake_get_token)
+
+    # Mock HTTP: first POST returns downloadUrl, then GET returns file bytes
+    file_content = b"fake file content"
+    channel._http = _FakeHttp(responses=[
+        _FakeResponse(200, {"downloadUrl": "https://example.com/tmpfile"}),
+        _FakeResponse(200),
+    ])
+    channel._http._responses[1].content = file_content
+
+    # Redirect media dir to tmp_path
+    monkeypatch.setattr(
+        "nanobot.config.paths.get_media_dir",
+        lambda channel_name=None: tmp_path / channel_name if channel_name else tmp_path,
+    )
+
+    result = await channel._download_dingtalk_file("code123", "test.xlsx", "user1")
+
+    assert result is not None
+    assert result.endswith("test.xlsx")
+    assert (tmp_path / "dingtalk" / "user1" / "test.xlsx").read_bytes() == file_content
+
+    # Verify API calls
+    assert channel._http.calls[0]["method"] == "POST"
+    assert "messageFiles/download" in channel._http.calls[0]["url"]
+    assert channel._http.calls[0]["json"]["downloadCode"] == "code123"
+    assert channel._http.calls[1]["method"] == "GET"

tests/test_email_channel.py

@@ -6,7 +6,7 @@ import pytest
 from nanobot.bus.events import OutboundMessage
 from nanobot.bus.queue import MessageBus
 from nanobot.channels.email import EmailChannel
-from nanobot.config.schema import EmailConfig
+from nanobot.channels.email import EmailConfig
 
 
 def _make_config() -> EmailConfig:

tests/test_evaluator.py

@@ -0,0 +1,63 @@
+import pytest
+
+from nanobot.utils.evaluator import evaluate_response
+from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest
+
+
+class DummyProvider(LLMProvider):
+    def __init__(self, responses: list[LLMResponse]):
+        super().__init__()
+        self._responses = list(responses)
+
+    async def chat(self, *args, **kwargs) -> LLMResponse:
+        if self._responses:
+            return self._responses.pop(0)
+        return LLMResponse(content="", tool_calls=[])
+
+    def get_default_model(self) -> str:
+        return "test-model"
+
+
+def _eval_tool_call(should_notify: bool, reason: str = "") -> LLMResponse:
+    return LLMResponse(
+        content="",
+        tool_calls=[
+            ToolCallRequest(
+                id="eval_1",
+                name="evaluate_notification",
+                arguments={"should_notify": should_notify, "reason": reason},
+            )
+        ],
+    )
+
+
+@pytest.mark.asyncio
+async def test_should_notify_true() -> None:
+    provider = DummyProvider([_eval_tool_call(True, "user asked to be reminded")])
+    result = await evaluate_response("Task completed with results", "check emails", provider, "m")
+    assert result is True
+
+
+@pytest.mark.asyncio
+async def test_should_notify_false() -> None:
+    provider = DummyProvider([_eval_tool_call(False, "routine check, nothing new")])
+    result = await evaluate_response("All clear, no updates", "check status", provider, "m")
+    assert result is False
+
+
+@pytest.mark.asyncio
+async def test_fallback_on_error() -> None:
+    class FailingProvider(DummyProvider):
+        async def chat(self, *args, **kwargs) -> LLMResponse:
+            raise RuntimeError("provider down")
+
+    provider = FailingProvider([])
+    result = await evaluate_response("some response", "some task", provider, "m")
+    assert result is True
+
+
+@pytest.mark.asyncio
+async def test_no_tool_call_fallback() -> None:
+    provider = DummyProvider([LLMResponse(content="I think you should notify", tool_calls=[])])
+    result = await evaluate_response("some response", "some task", provider, "m")
+    assert result is True

tests/test_exec_security.py

@@ -0,0 +1,69 @@
+"""Tests for exec tool internal URL blocking."""
+
+from __future__ import annotations
+
+import socket
+from unittest.mock import patch
+
+import pytest
+
+from nanobot.agent.tools.shell import ExecTool
+
+
+def _fake_resolve_private(hostname, port, family=0, type_=0):
+    return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("169.254.169.254", 0))]
+
+
+def _fake_resolve_localhost(hostname, port, family=0, type_=0):
+    return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 0))]
+
+
+def _fake_resolve_public(hostname, port, family=0, type_=0):
+    return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))]
+
+
+@pytest.mark.asyncio
+async def test_exec_blocks_curl_metadata():
+    tool = ExecTool()
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_private):
+        result = await tool.execute(
+            command='curl -s -H "Metadata-Flavor: Google" http://169.254.169.254/computeMetadata/v1/'
+        )
+    assert "Error" in result
+    assert "internal" in result.lower() or "private" in result.lower()
+
+
+@pytest.mark.asyncio
+async def test_exec_blocks_wget_localhost():
+    tool = ExecTool()
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_localhost):
+        result = await tool.execute(command="wget http://localhost:8080/secret -O /tmp/out")
+    assert "Error" in result
+
+
+@pytest.mark.asyncio
+async def test_exec_allows_normal_commands():
+    tool = ExecTool(timeout=5)
+    result = await tool.execute(command="echo hello")
+    assert "hello" in result
+    assert "Error" not in result.split("\n")[0]
+
+
+@pytest.mark.asyncio
+async def test_exec_allows_curl_to_public_url():
+    """Commands with public URLs should not be blocked by the internal URL check."""
+    tool = ExecTool()
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public):
+        guard_result = tool._guard_command("curl https://example.com/api", "/tmp")
+    assert guard_result is None
+
+
+@pytest.mark.asyncio
+async def test_exec_blocks_chained_internal_url():
+    """Internal URLs buried in chained commands should still be caught."""
+    tool = ExecTool()
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_private):
+        result = await tool.execute(
+            command="echo start && curl http://169.254.169.254/latest/meta-data/ && echo done"
+        )
+    assert "Error" in result

tests/test_feishu_reply.py

@@ -0,0 +1,392 @@
+"""Tests for Feishu message reply (quote) feature."""
+import asyncio
+import json
+from types import SimpleNamespace
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from nanobot.bus.events import OutboundMessage
+from nanobot.bus.queue import MessageBus
+from nanobot.channels.feishu import FeishuChannel, FeishuConfig
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+def _make_feishu_channel(reply_to_message: bool = False) -> FeishuChannel:
+    config = FeishuConfig(
+        enabled=True,
+        app_id="cli_test",
+        app_secret="secret",
+        allow_from=["*"],
+        reply_to_message=reply_to_message,
+    )
+    channel = FeishuChannel(config, MessageBus())
+    channel._client = MagicMock()
+    # _loop is only used by the WebSocket thread bridge; not needed for unit tests
+    channel._loop = None
+    return channel
+
+
+def _make_feishu_event(
+    *,
+    message_id: str = "om_001",
+    chat_id: str = "oc_abc",
+    chat_type: str = "p2p",
+    msg_type: str = "text",
+    content: str = '{"text": "hello"}',
+    sender_open_id: str = "ou_alice",
+    parent_id: str | None = None,
+    root_id: str | None = None,
+):
+    message = SimpleNamespace(
+        message_id=message_id,
+        chat_id=chat_id,
+        chat_type=chat_type,
+        message_type=msg_type,
+        content=content,
+        parent_id=parent_id,
+        root_id=root_id,
+        mentions=[],
+    )
+    sender = SimpleNamespace(
+        sender_type="user",
+        sender_id=SimpleNamespace(open_id=sender_open_id),
+    )
+    return SimpleNamespace(event=SimpleNamespace(message=message, sender=sender))
+
+
+def _make_get_message_response(text: str, msg_type: str = "text", success: bool = True):
+    """Build a fake im.v1.message.get response object."""
+    body = SimpleNamespace(content=json.dumps({"text": text}))
+    item = SimpleNamespace(msg_type=msg_type, body=body)
+    data = SimpleNamespace(items=[item])
+    resp = MagicMock()
+    resp.success.return_value = success
+    resp.data = data
+    resp.code = 0
+    resp.msg = "ok"
+    return resp
+
+
+# ---------------------------------------------------------------------------
+# Config tests
+# ---------------------------------------------------------------------------
+
+def test_feishu_config_reply_to_message_defaults_false() -> None:
+    assert FeishuConfig().reply_to_message is False
+
+
+def test_feishu_config_reply_to_message_can_be_enabled() -> None:
+    config = FeishuConfig(reply_to_message=True)
+    assert config.reply_to_message is True
+
+
+# ---------------------------------------------------------------------------
+# _get_message_content_sync tests
+# ---------------------------------------------------------------------------
+
+def test_get_message_content_sync_returns_reply_prefix() -> None:
+    channel = _make_feishu_channel()
+    channel._client.im.v1.message.get.return_value = _make_get_message_response("what time is it?")
+
+    result = channel._get_message_content_sync("om_parent")
+
+    assert result == "[Reply to: what time is it?]"
+
+
+def test_get_message_content_sync_truncates_long_text() -> None:
+    channel = _make_feishu_channel()
+    long_text = "x" * (FeishuChannel._REPLY_CONTEXT_MAX_LEN + 50)
+    channel._client.im.v1.message.get.return_value = _make_get_message_response(long_text)
+
+    result = channel._get_message_content_sync("om_parent")
+
+    assert result is not None
+    assert result.endswith("...]")
+    inner = result[len("[Reply to: ") : -1]
+    assert len(inner) == FeishuChannel._REPLY_CONTEXT_MAX_LEN + len("...")
+
+
+def test_get_message_content_sync_returns_none_on_api_failure() -> None:
+    channel = _make_feishu_channel()
+    resp = MagicMock()
+    resp.success.return_value = False
+    resp.code = 230002
+    resp.msg = "bot not in group"
+    channel._client.im.v1.message.get.return_value = resp
+
+    result = channel._get_message_content_sync("om_parent")
+
+    assert result is None
+
+
+def test_get_message_content_sync_returns_none_for_non_text_type() -> None:
+    channel = _make_feishu_channel()
+    body = SimpleNamespace(content=json.dumps({"image_key": "img_1"}))
+    item = SimpleNamespace(msg_type="image", body=body)
+    data = SimpleNamespace(items=[item])
+    resp = MagicMock()
+    resp.success.return_value = True
+    resp.data = data
+    channel._client.im.v1.message.get.return_value = resp
+
+    result = channel._get_message_content_sync("om_parent")
+
+    assert result is None
+
+
+def test_get_message_content_sync_returns_none_when_empty_text() -> None:
+    channel = _make_feishu_channel()
+    channel._client.im.v1.message.get.return_value = _make_get_message_response("   ")
+
+    result = channel._get_message_content_sync("om_parent")
+
+    assert result is None
+
+
+# ---------------------------------------------------------------------------
+# _reply_message_sync tests
+# ---------------------------------------------------------------------------
+
+def test_reply_message_sync_returns_true_on_success() -> None:
+    channel = _make_feishu_channel()
+    resp = MagicMock()
+    resp.success.return_value = True
+    channel._client.im.v1.message.reply.return_value = resp
+
+    ok = channel._reply_message_sync("om_parent", "text", '{"text":"hi"}')
+
+    assert ok is True
+    channel._client.im.v1.message.reply.assert_called_once()
+
+
+def test_reply_message_sync_returns_false_on_api_error() -> None:
+    channel = _make_feishu_channel()
+    resp = MagicMock()
+    resp.success.return_value = False
+    resp.code = 400
+    resp.msg = "bad request"
+    resp.get_log_id.return_value = "log_x"
+    channel._client.im.v1.message.reply.return_value = resp
+
+    ok = channel._reply_message_sync("om_parent", "text", '{"text":"hi"}')
+
+    assert ok is False
+
+
+def test_reply_message_sync_returns_false_on_exception() -> None:
+    channel = _make_feishu_channel()
+    channel._client.im.v1.message.reply.side_effect = RuntimeError("network error")
+
+    ok = channel._reply_message_sync("om_parent", "text", '{"text":"hi"}')
+
+    assert ok is False
+
+
+# ---------------------------------------------------------------------------
+# send() — reply routing tests
+# ---------------------------------------------------------------------------
+
+@pytest.mark.asyncio
+async def test_send_uses_reply_api_when_configured() -> None:
+    channel = _make_feishu_channel(reply_to_message=True)
+
+    reply_resp = MagicMock()
+    reply_resp.success.return_value = True
+    channel._client.im.v1.message.reply.return_value = reply_resp
+
+    await channel.send(OutboundMessage(
+        channel="feishu",
+        chat_id="oc_abc",
+        content="hello",
+        metadata={"message_id": "om_001"},
+    ))
+
+    channel._client.im.v1.message.reply.assert_called_once()
+    channel._client.im.v1.message.create.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_send_uses_create_api_when_reply_disabled() -> None:
+    channel = _make_feishu_channel(reply_to_message=False)
+
+    create_resp = MagicMock()
+    create_resp.success.return_value = True
+    channel._client.im.v1.message.create.return_value = create_resp
+
+    await channel.send(OutboundMessage(
+        channel="feishu",
+        chat_id="oc_abc",
+        content="hello",
+        metadata={"message_id": "om_001"},
+    ))
+
+    channel._client.im.v1.message.create.assert_called_once()
+    channel._client.im.v1.message.reply.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_send_uses_create_api_when_no_message_id() -> None:
+    channel = _make_feishu_channel(reply_to_message=True)
+
+    create_resp = MagicMock()
+    create_resp.success.return_value = True
+    channel._client.im.v1.message.create.return_value = create_resp
+
+    await channel.send(OutboundMessage(
+        channel="feishu",
+        chat_id="oc_abc",
+        content="hello",
+        metadata={},
+    ))
+
+    channel._client.im.v1.message.create.assert_called_once()
+    channel._client.im.v1.message.reply.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_send_skips_reply_for_progress_messages() -> None:
+    channel = _make_feishu_channel(reply_to_message=True)
+
+    create_resp = MagicMock()
+    create_resp.success.return_value = True
+    channel._client.im.v1.message.create.return_value = create_resp
+
+    await channel.send(OutboundMessage(
+        channel="feishu",
+        chat_id="oc_abc",
+        content="thinking...",
+        metadata={"message_id": "om_001", "_progress": True},
+    ))
+
+    channel._client.im.v1.message.create.assert_called_once()
+    channel._client.im.v1.message.reply.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_send_fallback_to_create_when_reply_fails() -> None:
+    channel = _make_feishu_channel(reply_to_message=True)
+
+    reply_resp = MagicMock()
+    reply_resp.success.return_value = False
+    reply_resp.code = 400
+    reply_resp.msg = "error"
+    reply_resp.get_log_id.return_value = "log_x"
+    channel._client.im.v1.message.reply.return_value = reply_resp
+
+    create_resp = MagicMock()
+    create_resp.success.return_value = True
+    channel._client.im.v1.message.create.return_value = create_resp
+
+    await channel.send(OutboundMessage(
+        channel="feishu",
+        chat_id="oc_abc",
+        content="hello",
+        metadata={"message_id": "om_001"},
+    ))
+
+    # reply attempted first, then falls back to create
+    channel._client.im.v1.message.reply.assert_called_once()
+    channel._client.im.v1.message.create.assert_called_once()
+
+
+# ---------------------------------------------------------------------------
+# _on_message — parent_id / root_id metadata tests
+# ---------------------------------------------------------------------------
+
+@pytest.mark.asyncio
+async def test_on_message_captures_parent_and_root_id_in_metadata() -> None:
+    channel = _make_feishu_channel()
+    channel._processed_message_ids.clear()
+    channel._client.im.v1.message.react.return_value = MagicMock(success=lambda: True)
+
+    captured = []
+
+    async def _capture(**kwargs):
+        captured.append(kwargs)
+
+    channel._handle_message = _capture
+
+    with patch.object(channel, "_add_reaction", return_value=None):
+        await channel._on_message(
+            _make_feishu_event(
+                parent_id="om_parent",
+                root_id="om_root",
+            )
+        )
+
+    assert len(captured) == 1
+    meta = captured[0]["metadata"]
+    assert meta["parent_id"] == "om_parent"
+    assert meta["root_id"] == "om_root"
+    assert meta["message_id"] == "om_001"
+
+
+@pytest.mark.asyncio
+async def test_on_message_parent_and_root_id_none_when_absent() -> None:
+    channel = _make_feishu_channel()
+    channel._processed_message_ids.clear()
+
+    captured = []
+
+    async def _capture(**kwargs):
+        captured.append(kwargs)
+
+    channel._handle_message = _capture
+
+    with patch.object(channel, "_add_reaction", return_value=None):
+        await channel._on_message(_make_feishu_event())
+
+    assert len(captured) == 1
+    meta = captured[0]["metadata"]
+    assert meta["parent_id"] is None
+    assert meta["root_id"] is None
+
+
+@pytest.mark.asyncio
+async def test_on_message_prepends_reply_context_when_parent_id_present() -> None:
+    channel = _make_feishu_channel()
+    channel._processed_message_ids.clear()
+    channel._client.im.v1.message.get.return_value = _make_get_message_response("original question")
+
+    captured = []
+
+    async def _capture(**kwargs):
+        captured.append(kwargs)
+
+    channel._handle_message = _capture
+
+    with patch.object(channel, "_add_reaction", return_value=None):
+        await channel._on_message(
+            _make_feishu_event(
+                content='{"text": "my answer"}',
+                parent_id="om_parent",
+            )
+        )
+
+    assert len(captured) == 1
+    content = captured[0]["content"]
+    assert content.startswith("[Reply to: original question]")
+    assert "my answer" in content
+
+
+@pytest.mark.asyncio
+async def test_on_message_no_extra_api_call_when_no_parent_id() -> None:
+    channel = _make_feishu_channel()
+    channel._processed_message_ids.clear()
+
+    captured = []
+
+    async def _capture(**kwargs):
+        captured.append(kwargs)
+
+    channel._handle_message = _capture
+
+    with patch.object(channel, "_add_reaction", return_value=None):
+        await channel._on_message(_make_feishu_event())
+
+    channel._client.im.v1.message.get.assert_not_called()
+    assert len(captured) == 1

tests/test_feishu_tool_hint_code_block.py

@@ -0,0 +1,138 @@
+"""Tests for FeishuChannel tool hint code block formatting."""
+
+import json
+from unittest.mock import MagicMock, patch
+
+import pytest
+from pytest import mark
+
+from nanobot.bus.events import OutboundMessage
+from nanobot.channels.feishu import FeishuChannel
+
+
+@pytest.fixture
+def mock_feishu_channel():
+    """Create a FeishuChannel with mocked client."""
+    config = MagicMock()
+    config.app_id = "test_app_id"
+    config.app_secret = "test_app_secret"
+    config.encrypt_key = None
+    config.verification_token = None
+    bus = MagicMock()
+    channel = FeishuChannel(config, bus)
+    channel._client = MagicMock()  # Simulate initialized client
+    return channel
+
+
+@mark.asyncio
+async def test_tool_hint_sends_code_message(mock_feishu_channel):
+    """Tool hint messages should be sent as interactive cards with code blocks."""
+    msg = OutboundMessage(
+        channel="feishu",
+        chat_id="oc_123456",
+        content='web_search("test query")',
+        metadata={"_tool_hint": True}
+    )
+
+    with patch.object(mock_feishu_channel, '_send_message_sync') as mock_send:
+        await mock_feishu_channel.send(msg)
+
+        # Verify interactive message with card was sent
+        assert mock_send.call_count == 1
+        call_args = mock_send.call_args[0]
+        receive_id_type, receive_id, msg_type, content = call_args
+
+        assert receive_id_type == "chat_id"
+        assert receive_id == "oc_123456"
+        assert msg_type == "interactive"
+
+        # Parse content to verify card structure
+        card = json.loads(content)
+        assert card["config"]["wide_screen_mode"] is True
+        assert len(card["elements"]) == 1
+        assert card["elements"][0]["tag"] == "markdown"
+        # Check that code block is properly formatted with language hint
+        expected_md = "**Tool Calls**\n\n```text\nweb_search(\"test query\")\n```"
+        assert card["elements"][0]["content"] == expected_md
+
+
+@mark.asyncio
+async def test_tool_hint_empty_content_does_not_send(mock_feishu_channel):
+    """Empty tool hint messages should not be sent."""
+    msg = OutboundMessage(
+        channel="feishu",
+        chat_id="oc_123456",
+        content="   ",  # whitespace only
+        metadata={"_tool_hint": True}
+    )
+
+    with patch.object(mock_feishu_channel, '_send_message_sync') as mock_send:
+        await mock_feishu_channel.send(msg)
+
+        # Should not send any message
+        mock_send.assert_not_called()
+
+
+@mark.asyncio
+async def test_tool_hint_without_metadata_sends_as_normal(mock_feishu_channel):
+    """Regular messages without _tool_hint should use normal formatting."""
+    msg = OutboundMessage(
+        channel="feishu",
+        chat_id="oc_123456",
+        content="Hello, world!",
+        metadata={}
+    )
+
+    with patch.object(mock_feishu_channel, '_send_message_sync') as mock_send:
+        await mock_feishu_channel.send(msg)
+
+        # Should send as text message (detected format)
+        assert mock_send.call_count == 1
+        call_args = mock_send.call_args[0]
+        _, _, msg_type, content = call_args
+        assert msg_type == "text"
+        assert json.loads(content) == {"text": "Hello, world!"}
+
+
+@mark.asyncio
+async def test_tool_hint_multiple_tools_in_one_message(mock_feishu_channel):
+    """Multiple tool calls should be displayed each on its own line in a code block."""
+    msg = OutboundMessage(
+        channel="feishu",
+        chat_id="oc_123456",
+        content='web_search("query"), read_file("/path/to/file")',
+        metadata={"_tool_hint": True}
+    )
+
+    with patch.object(mock_feishu_channel, '_send_message_sync') as mock_send:
+        await mock_feishu_channel.send(msg)
+
+        call_args = mock_send.call_args[0]
+        msg_type = call_args[2]
+        content = json.loads(call_args[3])
+        assert msg_type == "interactive"
+        # Each tool call should be on its own line
+        expected_md = "**Tool Calls**\n\n```text\nweb_search(\"query\"),\nread_file(\"/path/to/file\")\n```"
+        assert content["elements"][0]["content"] == expected_md
+
+
+@mark.asyncio
+async def test_tool_hint_keeps_commas_inside_arguments(mock_feishu_channel):
+    """Commas inside a single tool argument must not be split onto a new line."""
+    msg = OutboundMessage(
+        channel="feishu",
+        chat_id="oc_123456",
+        content='web_search("foo, bar"), read_file("/path/to/file")',
+        metadata={"_tool_hint": True}
+    )
+
+    with patch.object(mock_feishu_channel, '_send_message_sync') as mock_send:
+        await mock_feishu_channel.send(msg)
+
+        content = json.loads(mock_send.call_args[0][3])
+        expected_md = (
+            "**Tool Calls**\n\n```text\n"
+            "web_search(\"foo, bar\"),\n"
+            "read_file(\"/path/to/file\")\n```"
+        )
+        assert content["elements"][0]["content"] == expected_md

tests/test_filesystem_tools.py

@@ -222,8 +222,10 @@ class TestListDirTool:
     @pytest.mark.asyncio
     async def test_recursive(self, tool, populated_dir):
         result = await tool.execute(path=str(populated_dir), recursive=True)
-        assert "src/main.py" in result
-        assert "src/utils.py" in result
+        # Normalize path separators for cross-platform compatibility
+        normalized = result.replace("\\", "/")
+        assert "src/main.py" in normalized
+        assert "src/utils.py" in normalized
         assert "README.md" in result
         # Ignored dirs should not appear
         assert ".git" not in result
@@ -249,3 +251,114 @@ class TestListDirTool:
         result = await tool.execute(path=str(tmp_path / "nope"))
         assert "Error" in result
         assert "not found" in result
+
+
+# ---------------------------------------------------------------------------
+# Workspace restriction + extra_allowed_dirs
+# ---------------------------------------------------------------------------
+
+class TestWorkspaceRestriction:
+
+    @pytest.mark.asyncio
+    async def test_read_blocked_outside_workspace(self, tmp_path):
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        outside = tmp_path / "outside"
+        outside.mkdir()
+        secret = outside / "secret.txt"
+        secret.write_text("top secret")
+
+        tool = ReadFileTool(workspace=workspace, allowed_dir=workspace)
+        result = await tool.execute(path=str(secret))
+        assert "Error" in result
+        assert "outside" in result.lower()
+
+    @pytest.mark.asyncio
+    async def test_read_allowed_with_extra_dir(self, tmp_path):
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+        skill_file = skills_dir / "test_skill" / "SKILL.md"
+        skill_file.parent.mkdir()
+        skill_file.write_text("# Test Skill\nDo something.")
+
+        tool = ReadFileTool(
+            workspace=workspace, allowed_dir=workspace,
+            extra_allowed_dirs=[skills_dir],
+        )
+        result = await tool.execute(path=str(skill_file))
+        assert "Test Skill" in result
+        assert "Error" not in result
+
+    @pytest.mark.asyncio
+    async def test_extra_dirs_does_not_widen_write(self, tmp_path):
+        from nanobot.agent.tools.filesystem import WriteFileTool
+
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        outside = tmp_path / "outside"
+        outside.mkdir()
+
+        tool = WriteFileTool(workspace=workspace, allowed_dir=workspace)
+        result = await tool.execute(path=str(outside / "hack.txt"), content="pwned")
+        assert "Error" in result
+        assert "outside" in result.lower()
+
+    @pytest.mark.asyncio
+    async def test_read_still_blocked_for_unrelated_dir(self, tmp_path):
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+        unrelated = tmp_path / "other"
+        unrelated.mkdir()
+        secret = unrelated / "secret.txt"
+        secret.write_text("nope")
+
+        tool = ReadFileTool(
+            workspace=workspace, allowed_dir=workspace,
+            extra_allowed_dirs=[skills_dir],
+        )
+        result = await tool.execute(path=str(secret))
+        assert "Error" in result
+        assert "outside" in result.lower()
+
+    @pytest.mark.asyncio
+    async def test_workspace_file_still_readable_with_extra_dirs(self, tmp_path):
+        """Adding extra_allowed_dirs must not break normal workspace reads."""
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        ws_file = workspace / "README.md"
+        ws_file.write_text("hello from workspace")
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+
+        tool = ReadFileTool(
+            workspace=workspace, allowed_dir=workspace,
+            extra_allowed_dirs=[skills_dir],
+        )
+        result = await tool.execute(path=str(ws_file))
+        assert "hello from workspace" in result
+        assert "Error" not in result
+
+    @pytest.mark.asyncio
+    async def test_edit_blocked_in_extra_dir(self, tmp_path):
+        """edit_file must not be able to modify files in extra_allowed_dirs."""
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+        skill_file = skills_dir / "weather" / "SKILL.md"
+        skill_file.parent.mkdir()
+        skill_file.write_text("# Weather\nOriginal content.")
+
+        tool = EditFileTool(workspace=workspace, allowed_dir=workspace)
+        result = await tool.execute(
+            path=str(skill_file),
+            old_text="Original content.",
+            new_text="Hacked content.",
+        )
+        assert "Error" in result
+        assert "outside" in result.lower()
+        assert skill_file.read_text() == "# Weather\nOriginal content."

tests/test_heartbeat_service.py

@@ -123,6 +123,98 @@ async def test_trigger_now_returns_none_when_decision_is_skip(tmp_path) -> None:
     assert await service.trigger_now() is None
 
 
+@pytest.mark.asyncio
+async def test_tick_notifies_when_evaluator_says_yes(tmp_path, monkeypatch) -> None:
+    """Phase 1 run -> Phase 2 execute -> Phase 3 evaluate=notify -> on_notify called."""
+    (tmp_path / "HEARTBEAT.md").write_text("- [ ] check deployments", encoding="utf-8")
+
+    provider = DummyProvider([
+        LLMResponse(
+            content="",
+            tool_calls=[
+                ToolCallRequest(
+                    id="hb_1",
+                    name="heartbeat",
+                    arguments={"action": "run", "tasks": "check deployments"},
+                )
+            ],
+        ),
+    ])
+
+    executed: list[str] = []
+    notified: list[str] = []
+
+    async def _on_execute(tasks: str) -> str:
+        executed.append(tasks)
+        return "deployment failed on staging"
+
+    async def _on_notify(response: str) -> None:
+        notified.append(response)
+
+    service = HeartbeatService(
+        workspace=tmp_path,
+        provider=provider,
+        model="openai/gpt-4o-mini",
+        on_execute=_on_execute,
+        on_notify=_on_notify,
+    )
+
+    async def _eval_notify(*a, **kw):
+        return True
+
+    monkeypatch.setattr("nanobot.utils.evaluator.evaluate_response", _eval_notify)
+
+    await service._tick()
+    assert executed == ["check deployments"]
+    assert notified == ["deployment failed on staging"]
+
+
+@pytest.mark.asyncio
+async def test_tick_suppresses_when_evaluator_says_no(tmp_path, monkeypatch) -> None:
+    """Phase 1 run -> Phase 2 execute -> Phase 3 evaluate=silent -> on_notify NOT called."""
+    (tmp_path / "HEARTBEAT.md").write_text("- [ ] check status", encoding="utf-8")
+
+    provider = DummyProvider([
+        LLMResponse(
+            content="",
+            tool_calls=[
+                ToolCallRequest(
+                    id="hb_1",
+                    name="heartbeat",
+                    arguments={"action": "run", "tasks": "check status"},
+                )
+            ],
+        ),
+    ])
+
+    executed: list[str] = []
+    notified: list[str] = []
+
+    async def _on_execute(tasks: str) -> str:
+        executed.append(tasks)
+        return "everything is fine, no issues"
+
+    async def _on_notify(response: str) -> None:
+        notified.append(response)
+
+    service = HeartbeatService(
+        workspace=tmp_path,
+        provider=provider,
+        model="openai/gpt-4o-mini",
+        on_execute=_on_execute,
+        on_notify=_on_notify,
+    )
+
+    async def _eval_silent(*a, **kw):
+        return False
+
+    monkeypatch.setattr("nanobot.utils.evaluator.evaluate_response", _eval_silent)
+
+    await service._tick()
+    assert executed == ["check status"]
+    assert notified == []
+
+
 @pytest.mark.asyncio
 async def test_decide_retries_transient_error_then_succeeds(tmp_path, monkeypatch) -> None:
     provider = DummyProvider([
@@ -158,3 +250,40 @@ async def test_decide_retries_transient_error_then_succeeds(tmp_path, monkeypatc
     assert tasks == "check open tasks"
     assert provider.calls == 2
     assert delays == [1]
+
+
+@pytest.mark.asyncio
+async def test_decide_prompt_includes_current_time(tmp_path) -> None:
+    """Phase 1 user prompt must contain current time so the LLM can judge task urgency."""
+
+    captured_messages: list[dict] = []
+
+    class CapturingProvider(LLMProvider):
+        async def chat(self, *, messages=None, **kwargs) -> LLMResponse:
+            if messages:
+                captured_messages.extend(messages)
+            return LLMResponse(
+                content="",
+                tool_calls=[
+                    ToolCallRequest(
+                        id="hb_1", name="heartbeat",
+                        arguments={"action": "skip"},
+                    )
+                ],
+            )
+
+        def get_default_model(self) -> str:
+            return "test-model"
+
+    service = HeartbeatService(
+        workspace=tmp_path,
+        provider=CapturingProvider(),
+        model="test-model",
+    )
+
+    await service._decide("- [ ] check servers at 10:00 UTC")
+
+    user_msg = captured_messages[1]
+    assert user_msg["role"] == "user"
+    assert "Current Time:" in user_msg["content"]
+

tests/test_litellm_kwargs.py

@@ -0,0 +1,161 @@
+"""Regression tests for PR #2026 — litellm_kwargs injection from ProviderSpec.
+
+Validates that:
+- OpenRouter uses litellm_prefix (NOT custom_llm_provider) to avoid LiteLLM double-prefixing.
+- The litellm_kwargs mechanism works correctly for providers that declare it.
+- Non-gateway providers are unaffected.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+from typing import Any
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from nanobot.providers.litellm_provider import LiteLLMProvider
+from nanobot.providers.registry import find_by_name
+
+
+def _fake_response(content: str = "ok") -> SimpleNamespace:
+    """Build a minimal acompletion-shaped response object."""
+    message = SimpleNamespace(
+        content=content,
+        tool_calls=None,
+        reasoning_content=None,
+        thinking_blocks=None,
+    )
+    choice = SimpleNamespace(message=message, finish_reason="stop")
+    usage = SimpleNamespace(prompt_tokens=10, completion_tokens=5, total_tokens=15)
+    return SimpleNamespace(choices=[choice], usage=usage)
+
+
+def test_openrouter_spec_uses_prefix_not_custom_llm_provider() -> None:
+    """OpenRouter must rely on litellm_prefix, not custom_llm_provider kwarg.
+
+    LiteLLM internally adds a provider/ prefix when custom_llm_provider is set,
+    which double-prefixes models (openrouter/anthropic/model) and breaks the API.
+    """
+    spec = find_by_name("openrouter")
+    assert spec is not None
+    assert spec.litellm_prefix == "openrouter"
+    assert "custom_llm_provider" not in spec.litellm_kwargs, (
+        "custom_llm_provider causes LiteLLM to double-prefix the model name"
+    )
+
+
+@pytest.mark.asyncio
+async def test_openrouter_prefixes_model_correctly() -> None:
+    """OpenRouter should prefix model as openrouter/vendor/model for LiteLLM routing."""
+    mock_acompletion = AsyncMock(return_value=_fake_response())
+
+    with patch("nanobot.providers.litellm_provider.acompletion", mock_acompletion):
+        provider = LiteLLMProvider(
+            api_key="sk-or-test-key",
+            api_base="https://openrouter.ai/api/v1",
+            default_model="anthropic/claude-sonnet-4-5",
+            provider_name="openrouter",
+        )
+        await provider.chat(
+            messages=[{"role": "user", "content": "hello"}],
+            model="anthropic/claude-sonnet-4-5",
+        )
+
+    call_kwargs = mock_acompletion.call_args.kwargs
+    assert call_kwargs["model"] == "openrouter/anthropic/claude-sonnet-4-5", (
+        "LiteLLM needs openrouter/ prefix to detect the provider and strip it before API call"
+    )
+    assert "custom_llm_provider" not in call_kwargs
+
+
+@pytest.mark.asyncio
+async def test_non_gateway_provider_no_extra_kwargs() -> None:
+    """Standard (non-gateway) providers must NOT inject any litellm_kwargs."""
+    mock_acompletion = AsyncMock(return_value=_fake_response())
+
+    with patch("nanobot.providers.litellm_provider.acompletion", mock_acompletion):
+        provider = LiteLLMProvider(
+            api_key="sk-ant-test-key",
+            default_model="claude-sonnet-4-5",
+        )
+        await provider.chat(
+            messages=[{"role": "user", "content": "hello"}],
+            model="claude-sonnet-4-5",
+        )
+
+    call_kwargs = mock_acompletion.call_args.kwargs
+    assert "custom_llm_provider" not in call_kwargs, (
+        "Standard Anthropic provider should NOT inject custom_llm_provider"
+    )
+
+
+@pytest.mark.asyncio
+async def test_gateway_without_litellm_kwargs_injects_nothing_extra() -> None:
+    """Gateways without litellm_kwargs (e.g. AiHubMix) must not add extra keys."""
+    mock_acompletion = AsyncMock(return_value=_fake_response())
+
+    with patch("nanobot.providers.litellm_provider.acompletion", mock_acompletion):
+        provider = LiteLLMProvider(
+            api_key="sk-aihub-test-key",
+            api_base="https://aihubmix.com/v1",
+            default_model="claude-sonnet-4-5",
+            provider_name="aihubmix",
+        )
+        await provider.chat(
+            messages=[{"role": "user", "content": "hello"}],
+            model="claude-sonnet-4-5",
+        )
+
+    call_kwargs = mock_acompletion.call_args.kwargs
+    assert "custom_llm_provider" not in call_kwargs
+
+
+@pytest.mark.asyncio
+async def test_openrouter_autodetect_by_key_prefix() -> None:
+    """OpenRouter should be auto-detected by sk-or- key prefix even without explicit provider_name."""
+    mock_acompletion = AsyncMock(return_value=_fake_response())
+
+    with patch("nanobot.providers.litellm_provider.acompletion", mock_acompletion):
+        provider = LiteLLMProvider(
+            api_key="sk-or-auto-detect-key",
+            default_model="anthropic/claude-sonnet-4-5",
+        )
+        await provider.chat(
+            messages=[{"role": "user", "content": "hello"}],
+            model="anthropic/claude-sonnet-4-5",
+        )
+
+    call_kwargs = mock_acompletion.call_args.kwargs
+    assert call_kwargs["model"] == "openrouter/anthropic/claude-sonnet-4-5", (
+        "Auto-detected OpenRouter should prefix model for LiteLLM routing"
+    )
+
+
+@pytest.mark.asyncio
+async def test_openrouter_native_model_id_gets_double_prefixed() -> None:
+    """Models like openrouter/free must be double-prefixed so LiteLLM strips one layer.
+
+    openrouter/free is an actual OpenRouter model ID.  LiteLLM strips the first
+    openrouter/ for routing, so we must send openrouter/openrouter/free to ensure
+    the API receives openrouter/free.
+    """
+    mock_acompletion = AsyncMock(return_value=_fake_response())
+
+    with patch("nanobot.providers.litellm_provider.acompletion", mock_acompletion):
+        provider = LiteLLMProvider(
+            api_key="sk-or-test-key",
+            api_base="https://openrouter.ai/api/v1",
+            default_model="openrouter/free",
+            provider_name="openrouter",
+        )
+        await provider.chat(
+            messages=[{"role": "user", "content": "hello"}],
+            model="openrouter/free",
+        )
+
+    call_kwargs = mock_acompletion.call_args.kwargs
+    assert call_kwargs["model"] == "openrouter/openrouter/free", (
+        "openrouter/free must become openrouter/openrouter/free — "
+        "LiteLLM strips one layer so the API receives openrouter/free"
+    )

tests/test_matrix_channel.py

@@ -12,7 +12,7 @@ from nanobot.channels.matrix import (
     TYPING_NOTICE_TIMEOUT_MS,
     MatrixChannel,
 )
-from nanobot.config.schema import MatrixConfig
+from nanobot.channels.matrix import MatrixConfig
 
 _ROOM_SEND_UNSET = object()
 

tests/test_mcp_tool.py

@@ -1,12 +1,15 @@
 from __future__ import annotations
 
 import asyncio
+from contextlib import AsyncExitStack, asynccontextmanager
 import sys
 from types import ModuleType, SimpleNamespace
 
 import pytest
 
-from nanobot.agent.tools.mcp import MCPToolWrapper
+from nanobot.agent.tools.mcp import MCPToolWrapper, connect_mcp_servers
+from nanobot.agent.tools.registry import ToolRegistry
+from nanobot.config.schema import MCPServerConfig
 
 
 class _FakeTextContent:
@@ -14,12 +17,63 @@ class _FakeTextContent:
         self.text = text
 
 
+@pytest.fixture
+def fake_mcp_runtime() -> dict[str, object | None]:
+    return {"session": None}
+
+
 @pytest.fixture(autouse=True)
-def _fake_mcp_module(monkeypatch: pytest.MonkeyPatch) -> None:
+def _fake_mcp_module(
+    monkeypatch: pytest.MonkeyPatch, fake_mcp_runtime: dict[str, object | None]
+) -> None:
     mod = ModuleType("mcp")
     mod.types = SimpleNamespace(TextContent=_FakeTextContent)
+
+    class _FakeStdioServerParameters:
+        def __init__(self, command: str, args: list[str], env: dict | None = None) -> None:
+            self.command = command
+            self.args = args
+            self.env = env
+
+    class _FakeClientSession:
+        def __init__(self, _read: object, _write: object) -> None:
+            self._session = fake_mcp_runtime["session"]
+
+        async def __aenter__(self) -> object:
+            return self._session
+
+        async def __aexit__(self, exc_type, exc, tb) -> bool:
+            return False
+
+    @asynccontextmanager
+    async def _fake_stdio_client(_params: object):
+        yield object(), object()
+
+    @asynccontextmanager
+    async def _fake_sse_client(_url: str, httpx_client_factory=None):
+        yield object(), object()
+
+    @asynccontextmanager
+    async def _fake_streamable_http_client(_url: str, http_client=None):
+        yield object(), object(), object()
+
+    mod.ClientSession = _FakeClientSession
+    mod.StdioServerParameters = _FakeStdioServerParameters
     monkeypatch.setitem(sys.modules, "mcp", mod)
 
+    client_mod = ModuleType("mcp.client")
+    stdio_mod = ModuleType("mcp.client.stdio")
+    stdio_mod.stdio_client = _fake_stdio_client
+    sse_mod = ModuleType("mcp.client.sse")
+    sse_mod.sse_client = _fake_sse_client
+    streamable_http_mod = ModuleType("mcp.client.streamable_http")
+    streamable_http_mod.streamable_http_client = _fake_streamable_http_client
+
+    monkeypatch.setitem(sys.modules, "mcp.client", client_mod)
+    monkeypatch.setitem(sys.modules, "mcp.client.stdio", stdio_mod)
+    monkeypatch.setitem(sys.modules, "mcp.client.sse", sse_mod)
+    monkeypatch.setitem(sys.modules, "mcp.client.streamable_http", streamable_http_mod)
+
 
 def _make_wrapper(session: object, *, timeout: float = 0.1) -> MCPToolWrapper:
     tool_def = SimpleNamespace(
@@ -97,3 +151,132 @@ async def test_execute_handles_generic_exception() -> None:
     result = await wrapper.execute()
 
     assert result == "(MCP tool call failed: RuntimeError)"
+
+
+def _make_tool_def(name: str) -> SimpleNamespace:
+    return SimpleNamespace(
+        name=name,
+        description=f"{name} tool",
+        inputSchema={"type": "object", "properties": {}},
+    )
+
+
+def _make_fake_session(tool_names: list[str]) -> SimpleNamespace:
+    async def initialize() -> None:
+        return None
+
+    async def list_tools() -> SimpleNamespace:
+        return SimpleNamespace(tools=[_make_tool_def(name) for name in tool_names])
+
+    return SimpleNamespace(initialize=initialize, list_tools=list_tools)
+
+
+@pytest.mark.asyncio
+async def test_connect_mcp_servers_enabled_tools_supports_raw_names(
+    fake_mcp_runtime: dict[str, object | None],
+) -> None:
+    fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"])
+    registry = ToolRegistry()
+    stack = AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        await connect_mcp_servers(
+            {"test": MCPServerConfig(command="fake", enabled_tools=["demo"])},
+            registry,
+            stack,
+        )
+    finally:
+        await stack.aclose()
+
+    assert registry.tool_names == ["mcp_test_demo"]
+
+
+@pytest.mark.asyncio
+async def test_connect_mcp_servers_enabled_tools_defaults_to_all(
+    fake_mcp_runtime: dict[str, object | None],
+) -> None:
+    fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"])
+    registry = ToolRegistry()
+    stack = AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        await connect_mcp_servers(
+            {"test": MCPServerConfig(command="fake")},
+            registry,
+            stack,
+        )
+    finally:
+        await stack.aclose()
+
+    assert registry.tool_names == ["mcp_test_demo", "mcp_test_other"]
+
+
+@pytest.mark.asyncio
+async def test_connect_mcp_servers_enabled_tools_supports_wrapped_names(
+    fake_mcp_runtime: dict[str, object | None],
+) -> None:
+    fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"])
+    registry = ToolRegistry()
+    stack = AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        await connect_mcp_servers(
+            {"test": MCPServerConfig(command="fake", enabled_tools=["mcp_test_demo"])},
+            registry,
+            stack,
+        )
+    finally:
+        await stack.aclose()
+
+    assert registry.tool_names == ["mcp_test_demo"]
+
+
+@pytest.mark.asyncio
+async def test_connect_mcp_servers_enabled_tools_empty_list_registers_none(
+    fake_mcp_runtime: dict[str, object | None],
+) -> None:
+    fake_mcp_runtime["session"] = _make_fake_session(["demo", "other"])
+    registry = ToolRegistry()
+    stack = AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        await connect_mcp_servers(
+            {"test": MCPServerConfig(command="fake", enabled_tools=[])},
+            registry,
+            stack,
+        )
+    finally:
+        await stack.aclose()
+
+    assert registry.tool_names == []
+
+
+@pytest.mark.asyncio
+async def test_connect_mcp_servers_enabled_tools_warns_on_unknown_entries(
+    fake_mcp_runtime: dict[str, object | None], monkeypatch: pytest.MonkeyPatch
+) -> None:
+    fake_mcp_runtime["session"] = _make_fake_session(["demo"])
+    registry = ToolRegistry()
+    warnings: list[str] = []
+
+    def _warning(message: str, *args: object) -> None:
+        warnings.append(message.format(*args))
+
+    monkeypatch.setattr("nanobot.agent.tools.mcp.logger.warning", _warning)
+
+    stack = AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        await connect_mcp_servers(
+            {"test": MCPServerConfig(command="fake", enabled_tools=["unknown"])},
+            registry,
+            stack,
+        )
+    finally:
+        await stack.aclose()
+
+    assert registry.tool_names == []
+    assert warnings
+    assert "enabledTools entries not found: unknown" in warnings[-1]
+    assert "Available raw names: demo" in warnings[-1]
+    assert "Available wrapped names: mcp_test_demo" in warnings[-1]

tests/test_memory_consolidation_types.py

@@ -112,7 +112,6 @@ class TestMemoryConsolidationTypeHandling:
         store = MemoryStore(tmp_path)
         provider = AsyncMock()
 
-        # Simulate arguments being a JSON string (not yet parsed)
         response = LLMResponse(
             content=None,
             tool_calls=[
@@ -170,7 +169,6 @@ class TestMemoryConsolidationTypeHandling:
         store = MemoryStore(tmp_path)
         provider = AsyncMock()
 
-        # Simulate arguments being a list containing a dict
         response = LLMResponse(
             content=None,
             tool_calls=[
@@ -242,6 +240,94 @@ class TestMemoryConsolidationTypeHandling:
 
         assert result is False
 
+    @pytest.mark.asyncio
+    async def test_missing_history_entry_returns_false_without_writing(self, tmp_path: Path) -> None:
+        """Do not persist partial results when required fields are missing."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(
+            return_value=LLMResponse(
+                content=None,
+                tool_calls=[
+                    ToolCallRequest(
+                        id="call_1",
+                        name="save_memory",
+                        arguments={"memory_update": "# Memory\nOnly memory update"},
+                    )
+                ],
+            )
+        )
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is False
+        assert not store.history_file.exists()
+        assert not store.memory_file.exists()
+
+    @pytest.mark.asyncio
+    async def test_missing_memory_update_returns_false_without_writing(self, tmp_path: Path) -> None:
+        """Do not append history if memory_update is missing."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(
+            return_value=LLMResponse(
+                content=None,
+                tool_calls=[
+                    ToolCallRequest(
+                        id="call_1",
+                        name="save_memory",
+                        arguments={"history_entry": "[2026-01-01] Partial output."},
+                    )
+                ],
+            )
+        )
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is False
+        assert not store.history_file.exists()
+        assert not store.memory_file.exists()
+
+    @pytest.mark.asyncio
+    async def test_null_required_field_returns_false_without_writing(self, tmp_path: Path) -> None:
+        """Null required fields should be rejected before persistence."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(
+            return_value=_make_tool_response(
+                history_entry=None,
+                memory_update="# Memory\nUser likes testing.",
+            )
+        )
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is False
+        assert not store.history_file.exists()
+        assert not store.memory_file.exists()
+
+    @pytest.mark.asyncio
+    async def test_empty_history_entry_returns_false_without_writing(self, tmp_path: Path) -> None:
+        """Empty history entries should be rejected to avoid blank archival records."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(
+            return_value=_make_tool_response(
+                history_entry="   ",
+                memory_update="# Memory\nUser likes testing.",
+            )
+        )
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is False
+        assert not store.history_file.exists()
+        assert not store.memory_file.exists()
+
     @pytest.mark.asyncio
     async def test_retries_transient_error_then_succeeds(self, tmp_path: Path, monkeypatch) -> None:
         store = MemoryStore(tmp_path)
@@ -288,3 +374,105 @@ class TestMemoryConsolidationTypeHandling:
         assert "temperature" not in kwargs
         assert "max_tokens" not in kwargs
         assert "reasoning_effort" not in kwargs
+
+    @pytest.mark.asyncio
+    async def test_tool_choice_fallback_on_unsupported_error(self, tmp_path: Path) -> None:
+        """Forced tool_choice rejected by provider -> retry with auto and succeed."""
+        store = MemoryStore(tmp_path)
+        error_resp = LLMResponse(
+            content="Error calling LLM: litellm.BadRequestError: "
+            "The tool_choice parameter does not support being set to required or object",
+            finish_reason="error",
+            tool_calls=[],
+        )
+        ok_resp = _make_tool_response(
+            history_entry="[2026-01-01] Fallback worked.",
+            memory_update="# Memory\nFallback OK.",
+        )
+
+        call_log: list[dict] = []
+
+        async def _tracking_chat(**kwargs):
+            call_log.append(kwargs)
+            return error_resp if len(call_log) == 1 else ok_resp
+
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(side_effect=_tracking_chat)
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is True
+        assert len(call_log) == 2
+        assert isinstance(call_log[0]["tool_choice"], dict)
+        assert call_log[1]["tool_choice"] == "auto"
+        assert "Fallback worked." in store.history_file.read_text()
+
+    @pytest.mark.asyncio
+    async def test_tool_choice_fallback_auto_no_tool_call(self, tmp_path: Path) -> None:
+        """Forced rejected, auto retry also produces no tool call -> return False."""
+        store = MemoryStore(tmp_path)
+        error_resp = LLMResponse(
+            content="Error: tool_choice must be none or auto",
+            finish_reason="error",
+            tool_calls=[],
+        )
+        no_tool_resp = LLMResponse(
+            content="Here is a summary.",
+            finish_reason="stop",
+            tool_calls=[],
+        )
+
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(side_effect=[error_resp, no_tool_resp])
+        messages = _make_messages(message_count=60)
+
+        result = await store.consolidate(messages, provider, "test-model")
+
+        assert result is False
+        assert not store.history_file.exists()
+
+    @pytest.mark.asyncio
+    async def test_raw_archive_after_consecutive_failures(self, tmp_path: Path) -> None:
+        """After 3 consecutive failures, raw-archive messages and return True."""
+        store = MemoryStore(tmp_path)
+        no_tool = LLMResponse(content="No tool call.", finish_reason="stop", tool_calls=[])
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(return_value=no_tool)
+        messages = _make_messages(message_count=10)
+
+        assert await store.consolidate(messages, provider, "m") is False
+        assert await store.consolidate(messages, provider, "m") is False
+        assert await store.consolidate(messages, provider, "m") is True
+
+        assert store.history_file.exists()
+        content = store.history_file.read_text()
+        assert "[RAW]" in content
+        assert "10 messages" in content
+        assert "msg0" in content
+        assert not store.memory_file.exists()
+
+    @pytest.mark.asyncio
+    async def test_raw_archive_counter_resets_on_success(self, tmp_path: Path) -> None:
+        """A successful consolidation resets the failure counter."""
+        store = MemoryStore(tmp_path)
+        no_tool = LLMResponse(content="Nope.", finish_reason="stop", tool_calls=[])
+        ok_resp = _make_tool_response(
+            history_entry="[2026-01-01] OK.",
+            memory_update="# Memory\nOK.",
+        )
+        messages = _make_messages(message_count=10)
+
+        provider = AsyncMock()
+        provider.chat_with_retry = AsyncMock(return_value=no_tool)
+        assert await store.consolidate(messages, provider, "m") is False
+        assert await store.consolidate(messages, provider, "m") is False
+        assert store._consecutive_failures == 2
+
+        provider.chat_with_retry = AsyncMock(return_value=ok_resp)
+        assert await store.consolidate(messages, provider, "m") is True
+        assert store._consecutive_failures == 0
+
+        provider.chat_with_retry = AsyncMock(return_value=no_tool)
+        assert await store.consolidate(messages, provider, "m") is False
+        assert store._consecutive_failures == 1

tests/test_provider_retry.py

@@ -123,3 +123,87 @@ async def test_chat_with_retry_explicit_override_beats_defaults() -> None:
     assert provider.last_kwargs["temperature"] == 0.9
     assert provider.last_kwargs["max_tokens"] == 9999
     assert provider.last_kwargs["reasoning_effort"] == "low"
+
+
+# ---------------------------------------------------------------------------
+# Image-unsupported fallback tests
+# ---------------------------------------------------------------------------
+
+_IMAGE_MSG = [
+    {"role": "user", "content": [
+        {"type": "text", "text": "describe this"},
+        {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}},
+    ]},
+]
+
+
+@pytest.mark.asyncio
+async def test_image_unsupported_error_retries_without_images() -> None:
+    """If the model rejects image_url, retry once with images stripped."""
+    provider = ScriptedProvider([
+        LLMResponse(
+            content="Invalid content type. image_url is only supported by certain models",
+            finish_reason="error",
+        ),
+        LLMResponse(content="ok, no image"),
+    ])
+
+    response = await provider.chat_with_retry(messages=_IMAGE_MSG)
+
+    assert response.content == "ok, no image"
+    assert provider.calls == 2
+    msgs_on_retry = provider.last_kwargs["messages"]
+    for msg in msgs_on_retry:
+        content = msg.get("content")
+        if isinstance(content, list):
+            assert all(b.get("type") != "image_url" for b in content)
+            assert any("[image omitted]" in (b.get("text") or "") for b in content)
+
+
+@pytest.mark.asyncio
+async def test_image_unsupported_error_no_retry_without_image_content() -> None:
+    """If messages don't contain image_url blocks, don't retry on image error."""
+    provider = ScriptedProvider([
+        LLMResponse(
+            content="image_url is only supported by certain models",
+            finish_reason="error",
+        ),
+    ])
+
+    response = await provider.chat_with_retry(
+        messages=[{"role": "user", "content": "hello"}],
+    )
+
+    assert provider.calls == 1
+    assert response.finish_reason == "error"
+
+
+@pytest.mark.asyncio
+async def test_image_unsupported_fallback_returns_error_on_second_failure() -> None:
+    """If the image-stripped retry also fails, return that error."""
+    provider = ScriptedProvider([
+        LLMResponse(
+            content="does not support image input",
+            finish_reason="error",
+        ),
+        LLMResponse(content="some other error", finish_reason="error"),
+    ])
+
+    response = await provider.chat_with_retry(messages=_IMAGE_MSG)
+
+    assert provider.calls == 2
+    assert response.content == "some other error"
+    assert response.finish_reason == "error"
+
+
+@pytest.mark.asyncio
+async def test_non_image_error_does_not_trigger_image_fallback() -> None:
+    """Regular non-transient errors must not trigger image stripping."""
+    provider = ScriptedProvider([
+        LLMResponse(content="401 unauthorized", finish_reason="error"),
+    ])
+
+    response = await provider.chat_with_retry(messages=_IMAGE_MSG)
+
+    assert provider.calls == 1
+    assert response.content == "401 unauthorized"

tests/test_qq_channel.py

@@ -5,7 +5,7 @@ import pytest
 from nanobot.bus.events import OutboundMessage
 from nanobot.bus.queue import MessageBus
 from nanobot.channels.qq import QQChannel
-from nanobot.config.schema import QQConfig
+from nanobot.channels.qq import QQConfig
 
 
 class _FakeApi:
@@ -44,7 +44,7 @@ async def test_on_group_message_routes_to_group_chat_id() -> None:
 
 
 @pytest.mark.asyncio
-async def test_send_group_message_uses_group_api_with_msg_seq() -> None:
+async def test_send_group_message_uses_plain_text_group_api_with_msg_seq() -> None:
     channel = QQChannel(QQConfig(app_id="app", secret="secret", allow_from=["*"]), MessageBus())
     channel._client = _FakeClient()
     channel._chat_type_cache["group123"] = "group"
@@ -60,7 +60,66 @@ async def test_send_group_message_uses_group_api_with_msg_seq() -> None:
 
     assert len(channel._client.api.group_calls) == 1
     call = channel._client.api.group_calls[0]
-    assert call["group_openid"] == "group123"
-    assert call["msg_id"] == "msg1"
-    assert call["msg_seq"] == 2
+    assert call == {
+        "group_openid": "group123",
+        "msg_type": 0,
+        "content": "hello",
+        "msg_id": "msg1",
+        "msg_seq": 2,
+    }
     assert not channel._client.api.c2c_calls
+
+
+@pytest.mark.asyncio
+async def test_send_c2c_message_uses_plain_text_c2c_api_with_msg_seq() -> None:
+    channel = QQChannel(QQConfig(app_id="app", secret="secret", allow_from=["*"]), MessageBus())
+    channel._client = _FakeClient()
+
+    await channel.send(
+        OutboundMessage(
+            channel="qq",
+            chat_id="user123",
+            content="hello",
+            metadata={"message_id": "msg1"},
+        )
+    )
+
+    assert len(channel._client.api.c2c_calls) == 1
+    call = channel._client.api.c2c_calls[0]
+    assert call == {
+        "openid": "user123",
+        "msg_type": 0,
+        "content": "hello",
+        "msg_id": "msg1",
+        "msg_seq": 2,
+    }
+    assert not channel._client.api.group_calls
+
+
+@pytest.mark.asyncio
+async def test_send_group_message_uses_markdown_when_configured() -> None:
+    channel = QQChannel(
+        QQConfig(app_id="app", secret="secret", allow_from=["*"], msg_format="markdown"),
+        MessageBus(),
+    )
+    channel._client = _FakeClient()
+    channel._chat_type_cache["group123"] = "group"
+
+    await channel.send(
+        OutboundMessage(
+            channel="qq",
+            chat_id="group123",
+            content="**hello**",
+            metadata={"message_id": "msg1"},
+        )
+    )
+
+    assert len(channel._client.api.group_calls) == 1
+    call = channel._client.api.group_calls[0]
+    assert call == {
+        "group_openid": "group123",
+        "msg_type": 2,
+        "markdown": {"content": "**hello**"},
+        "msg_id": "msg1",
+        "msg_seq": 2,
+    }

tests/test_security_network.py

@@ -0,0 +1,101 @@
+"""Tests for nanobot.security.network — SSRF protection and internal URL detection."""
+
+from __future__ import annotations
+
+import socket
+from unittest.mock import patch
+
+import pytest
+
+from nanobot.security.network import contains_internal_url, validate_url_target
+
+
+def _fake_resolve(host: str, results: list[str]):
+    """Return a getaddrinfo mock that maps the given host to fake IP results."""
+    def _resolver(hostname, port, family=0, type_=0):
+        if hostname == host:
+            return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", (ip, 0)) for ip in results]
+        raise socket.gaierror(f"cannot resolve {hostname}")
+    return _resolver
+
+
+# ---------------------------------------------------------------------------
+# validate_url_target — scheme / domain basics
+# ---------------------------------------------------------------------------
+
+def test_rejects_non_http_scheme():
+    ok, err = validate_url_target("ftp://example.com/file")
+    assert not ok
+    assert "http" in err.lower()
+
+
+def test_rejects_missing_domain():
+    ok, err = validate_url_target("http://")
+    assert not ok
+
+
+# ---------------------------------------------------------------------------
+# validate_url_target — blocked private/internal IPs
+# ---------------------------------------------------------------------------
+
+@pytest.mark.parametrize("ip,label", [
+    ("127.0.0.1", "loopback"),
+    ("127.0.0.2", "loopback_alt"),
+    ("10.0.0.1", "rfc1918_10"),
+    ("172.16.5.1", "rfc1918_172"),
+    ("192.168.1.1", "rfc1918_192"),
+    ("169.254.169.254", "metadata"),
+    ("0.0.0.0", "zero"),
+])
+def test_blocks_private_ipv4(ip: str, label: str):
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("evil.com", [ip])):
+        ok, err = validate_url_target(f"http://evil.com/path")
+        assert not ok, f"Should block {label} ({ip})"
+        assert "private" in err.lower() or "blocked" in err.lower()
+
+
+def test_blocks_ipv6_loopback():
+    def _resolver(hostname, port, family=0, type_=0):
+        return [(socket.AF_INET6, socket.SOCK_STREAM, 0, "", ("::1", 0, 0, 0))]
+    with patch("nanobot.security.network.socket.getaddrinfo", _resolver):
+        ok, err = validate_url_target("http://evil.com/")
+        assert not ok
+
+
+# ---------------------------------------------------------------------------
+# validate_url_target — allows public IPs
+# ---------------------------------------------------------------------------
+
+def test_allows_public_ip():
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("example.com", ["93.184.216.34"])):
+        ok, err = validate_url_target("http://example.com/page")
+        assert ok, f"Should allow public IP, got: {err}"
+
+
+def test_allows_normal_https():
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("github.com", ["140.82.121.3"])):
+        ok, err = validate_url_target("https://github.com/HKUDS/nanobot")
+        assert ok
+
+
+# ---------------------------------------------------------------------------
+# contains_internal_url — shell command scanning
+# ---------------------------------------------------------------------------
+
+def test_detects_curl_metadata():
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("169.254.169.254", ["169.254.169.254"])):
+        assert contains_internal_url('curl -s http://169.254.169.254/computeMetadata/v1/')
+
+
+def test_detects_wget_localhost():
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("localhost", ["127.0.0.1"])):
+        assert contains_internal_url("wget http://localhost:8080/secret")
+
+
+def test_allows_normal_curl():
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve("example.com", ["93.184.216.34"])):
+        assert not contains_internal_url("curl https://example.com/api/data")
+
+
+def test_no_urls_returns_false():
+    assert not contains_internal_url("echo hello && ls -la")

tests/test_slack_channel.py

@@ -5,7 +5,7 @@ import pytest
 from nanobot.bus.events import OutboundMessage
 from nanobot.bus.queue import MessageBus
 from nanobot.channels.slack import SlackChannel
-from nanobot.config.schema import SlackConfig
+from nanobot.channels.slack import SlackConfig
 
 
 class _FakeAsyncWebClient:

tests/test_telegram_channel.py

@@ -8,7 +8,7 @@ import pytest
 from nanobot.bus.events import OutboundMessage
 from nanobot.bus.queue import MessageBus
 from nanobot.channels.telegram import TELEGRAM_REPLY_CONTEXT_MAX_LEN, TelegramChannel
-from nanobot.config.schema import TelegramConfig
+from nanobot.channels.telegram import TelegramConfig
 
 
 class _FakeHTTPXRequest:
@@ -446,6 +446,56 @@ async def test_download_message_media_returns_path_when_download_succeeds(
     assert "[image:" in parts[0]
 
 
+@pytest.mark.asyncio
+async def test_download_message_media_uses_file_unique_id_when_available(
+    monkeypatch, tmp_path
+) -> None:
+    media_dir = tmp_path / "media" / "telegram"
+    media_dir.mkdir(parents=True)
+    monkeypatch.setattr(
+        "nanobot.channels.telegram.get_media_dir",
+        lambda channel=None: media_dir if channel else tmp_path / "media",
+    )
+
+    downloaded: dict[str, str] = {}
+
+    async def _download_to_drive(path: str) -> None:
+        downloaded["path"] = path
+
+    channel = TelegramChannel(
+        TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]),
+        MessageBus(),
+    )
+    app = _FakeApp(lambda: None)
+    app.bot.get_file = AsyncMock(
+        return_value=SimpleNamespace(download_to_drive=_download_to_drive)
+    )
+    channel._app = app
+
+    msg = SimpleNamespace(
+        photo=[
+            SimpleNamespace(
+                file_id="file-id-that-should-not-be-used",
+                file_unique_id="stable-unique-id",
+                mime_type="image/jpeg",
+                file_name=None,
+            )
+        ],
+        voice=None,
+        audio=None,
+        document=None,
+        video=None,
+        video_note=None,
+        animation=None,
+    )
+
+    paths, parts = await channel._download_message_media(msg)
+
+    assert downloaded["path"].endswith("stable-unique-id.jpg")
+    assert paths == [str(media_dir / "stable-unique-id.jpg")]
+    assert parts == [f"[image: {media_dir / 'stable-unique-id.jpg'}]"]
+
+
 @pytest.mark.asyncio
 async def test_on_message_attaches_reply_to_media_when_available(monkeypatch, tmp_path) -> None:
     """When user replies to a message with media, that media is downloaded and attached to the turn."""
@@ -597,3 +647,19 @@ async def test_forward_command_does_not_inject_reply_context() -> None:
 
     assert len(handled) == 1
     assert handled[0]["content"] == "/new"
+
+
+@pytest.mark.asyncio
+async def test_on_help_includes_restart_command() -> None:
+    channel = TelegramChannel(
+        TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], group_policy="open"),
+        MessageBus(),
+    )
+    update = _make_telegram_update(text="/help", chat_type="private")
+    update.message.reply_text = AsyncMock()
+
+    await channel._on_help(update, None)
+
+    update.message.reply_text.assert_awaited_once()
+    help_text = update.message.reply_text.await_args.args[0]
+    assert "/restart" in help_text

tests/test_tool_validation.py

@@ -379,9 +379,11 @@ async def test_exec_always_returns_exit_code() -> None:
 async def test_exec_head_tail_truncation() -> None:
     """Long output should preserve both head and tail."""
     tool = ExecTool()
-    # Generate output that exceeds _MAX_OUTPUT
-    big = "A" * 6000 + "\n" + "B" * 6000
-    result = await tool.execute(command=f"echo '{big}'")
+    # Generate output that exceeds _MAX_OUTPUT (10_000 chars)
+    # Use python to generate output to avoid command line length limits
+    result = await tool.execute(
+        command="python -c \"print('A' * 6000 + '\\n' + 'B' * 6000)\""
+    )
     assert "chars truncated" in result
     # Head portion should start with As
     assert result.startswith("A")

tests/test_web_fetch_security.py

@@ -0,0 +1,69 @@
+"""Tests for web_fetch SSRF protection and untrusted content marking."""
+
+from __future__ import annotations
+
+import json
+import socket
+from unittest.mock import patch
+
+import pytest
+
+from nanobot.agent.tools.web import WebFetchTool
+
+
+def _fake_resolve_private(hostname, port, family=0, type_=0):
+    return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("169.254.169.254", 0))]
+
+
+def _fake_resolve_public(hostname, port, family=0, type_=0):
+    return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))]
+
+
+@pytest.mark.asyncio
+async def test_web_fetch_blocks_private_ip():
+    tool = WebFetchTool()
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_private):
+        result = await tool.execute(url="http://169.254.169.254/computeMetadata/v1/")
+    data = json.loads(result)
+    assert "error" in data
+    assert "private" in data["error"].lower() or "blocked" in data["error"].lower()
+
+
+@pytest.mark.asyncio
+async def test_web_fetch_blocks_localhost():
+    tool = WebFetchTool()
+    def _resolve_localhost(hostname, port, family=0, type_=0):
+        return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 0))]
+    with patch("nanobot.security.network.socket.getaddrinfo", _resolve_localhost):
+        result = await tool.execute(url="http://localhost/admin")
+    data = json.loads(result)
+    assert "error" in data
+
+
+@pytest.mark.asyncio
+async def test_web_fetch_result_contains_untrusted_flag():
+    """When fetch succeeds, result JSON must include untrusted=True and the banner."""
+    tool = WebFetchTool()
+
+    fake_html = "<html><head><title>Test</title></head><body><p>Hello world</p></body></html>"
+
+    import httpx
+
+    class FakeResponse:
+        status_code = 200
+        url = "https://example.com/page"
+        text = fake_html
+        headers = {"content-type": "text/html"}
+        def raise_for_status(self): pass
+        def json(self): return {}
+
+    async def _fake_get(self, url, **kwargs):
+        return FakeResponse()
+
+    with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public), \
+         patch("httpx.AsyncClient.get", _fake_get):
+        result = await tool.execute(url="https://example.com/page")
+
+    data = json.loads(result)
+    assert data.get("untrusted") is True
+    assert "[External content" in data.get("text", "")

tests/test_web_search_tool.py

@@ -0,0 +1,162 @@
+"""Tests for multi-provider web search."""
+
+import httpx
+import pytest
+
+from nanobot.agent.tools.web import WebSearchTool
+from nanobot.config.schema import WebSearchConfig
+
+
+def _tool(provider: str = "brave", api_key: str = "", base_url: str = "") -> WebSearchTool:
+    return WebSearchTool(config=WebSearchConfig(provider=provider, api_key=api_key, base_url=base_url))
+
+
+def _response(status: int = 200, json: dict | None = None) -> httpx.Response:
+    """Build a mock httpx.Response with a dummy request attached."""
+    r = httpx.Response(status, json=json)
+    r._request = httpx.Request("GET", "https://mock")
+    return r
+
+
+@pytest.mark.asyncio
+async def test_brave_search(monkeypatch):
+    async def mock_get(self, url, **kw):
+        assert "brave" in url
+        assert kw["headers"]["X-Subscription-Token"] == "brave-key"
+        return _response(json={
+            "web": {"results": [{"title": "NanoBot", "url": "https://example.com", "description": "AI assistant"}]}
+        })
+
+    monkeypatch.setattr(httpx.AsyncClient, "get", mock_get)
+    tool = _tool(provider="brave", api_key="brave-key")
+    result = await tool.execute(query="nanobot", count=1)
+    assert "NanoBot" in result
+    assert "https://example.com" in result
+
+
+@pytest.mark.asyncio
+async def test_tavily_search(monkeypatch):
+    async def mock_post(self, url, **kw):
+        assert "tavily" in url
+        assert kw["headers"]["Authorization"] == "Bearer tavily-key"
+        return _response(json={
+            "results": [{"title": "OpenClaw", "url": "https://openclaw.io", "content": "Framework"}]
+        })
+
+    monkeypatch.setattr(httpx.AsyncClient, "post", mock_post)
+    tool = _tool(provider="tavily", api_key="tavily-key")
+    result = await tool.execute(query="openclaw")
+    assert "OpenClaw" in result
+    assert "https://openclaw.io" in result
+
+
+@pytest.mark.asyncio
+async def test_searxng_search(monkeypatch):
+    async def mock_get(self, url, **kw):
+        assert "searx.example" in url
+        return _response(json={
+            "results": [{"title": "Result", "url": "https://example.com", "content": "SearXNG result"}]
+        })
+
+    monkeypatch.setattr(httpx.AsyncClient, "get", mock_get)
+    tool = _tool(provider="searxng", base_url="https://searx.example")
+    result = await tool.execute(query="test")
+    assert "Result" in result
+
+
+@pytest.mark.asyncio
+async def test_duckduckgo_search(monkeypatch):
+    class MockDDGS:
+        def __init__(self, **kw):
+            pass
+
+        def text(self, query, max_results=5):
+            return [{"title": "DDG Result", "href": "https://ddg.example", "body": "From DuckDuckGo"}]
+
+    monkeypatch.setattr("nanobot.agent.tools.web.DDGS", MockDDGS, raising=False)
+    import nanobot.agent.tools.web as web_mod
+    monkeypatch.setattr(web_mod, "DDGS", MockDDGS, raising=False)
+
+    from ddgs import DDGS
+    monkeypatch.setattr("ddgs.DDGS", MockDDGS)
+
+    tool = _tool(provider="duckduckgo")
+    result = await tool.execute(query="hello")
+    assert "DDG Result" in result
+
+
+@pytest.mark.asyncio
+async def test_brave_fallback_to_duckduckgo_when_no_key(monkeypatch):
+    class MockDDGS:
+        def __init__(self, **kw):
+            pass
+
+        def text(self, query, max_results=5):
+            return [{"title": "Fallback", "href": "https://ddg.example", "body": "DuckDuckGo fallback"}]
+
+    monkeypatch.setattr("ddgs.DDGS", MockDDGS)
+    monkeypatch.delenv("BRAVE_API_KEY", raising=False)
+
+    tool = _tool(provider="brave", api_key="")
+    result = await tool.execute(query="test")
+    assert "Fallback" in result
+
+
+@pytest.mark.asyncio
+async def test_jina_search(monkeypatch):
+    async def mock_get(self, url, **kw):
+        assert "s.jina.ai" in str(url)
+        assert kw["headers"]["Authorization"] == "Bearer jina-key"
+        return _response(json={
+            "data": [{"title": "Jina Result", "url": "https://jina.ai", "content": "AI search"}]
+        })
+
+    monkeypatch.setattr(httpx.AsyncClient, "get", mock_get)
+    tool = _tool(provider="jina", api_key="jina-key")
+    result = await tool.execute(query="test")
+    assert "Jina Result" in result
+    assert "https://jina.ai" in result
+
+
+@pytest.mark.asyncio
+async def test_unknown_provider():
+    tool = _tool(provider="unknown")
+    result = await tool.execute(query="test")
+    assert "unknown" in result
+    assert "Error" in result
+
+
+@pytest.mark.asyncio
+async def test_default_provider_is_brave(monkeypatch):
+    async def mock_get(self, url, **kw):
+        assert "brave" in url
+        return _response(json={"web": {"results": []}})
+
+    monkeypatch.setattr(httpx.AsyncClient, "get", mock_get)
+    tool = _tool(provider="", api_key="test-key")
+    result = await tool.execute(query="test")
+    assert "No results" in result
+
+
+@pytest.mark.asyncio
+async def test_searxng_no_base_url_falls_back(monkeypatch):
+    class MockDDGS:
+        def __init__(self, **kw):
+            pass
+
+        def text(self, query, max_results=5):
+            return [{"title": "Fallback", "href": "https://ddg.example", "body": "fallback"}]
+
+    monkeypatch.setattr("ddgs.DDGS", MockDDGS)
+    monkeypatch.delenv("SEARXNG_BASE_URL", raising=False)
+
+    tool = _tool(provider="searxng", base_url="")
+    result = await tool.execute(query="test")
+    assert "Fallback" in result
+
+
+@pytest.mark.asyncio
+async def test_searxng_invalid_url():
+    tool = _tool(provider="searxng", base_url="not-a-url")
+    result = await tool.execute(query="test")
+    assert "Error" in result