Merge remote-tracking branch 'origin/main' into feat/openai-compatible-session-isolation

# Conflicts:
#	nanobot/agent/context.py
#	tests/test_consolidate_offset.py

tests/test_consolidate_offset.py

@@ -786,10 +786,8 @@ class TestConsolidationDeduplicationGuard:
         )
 
     @pytest.mark.asyncio
-    async def test_new_cleans_up_consolidation_lock_for_invalidated_session(
-        self, tmp_path: Path
-    ) -> None:
-        """/new should remove lock entry for fully invalidated session key."""
+    async def test_new_clears_session_and_responds(self, tmp_path: Path) -> None:
+        """/new clears session and returns confirmation."""
         from nanobot.agent.loop import AgentLoop
         from nanobot.bus.events import InboundMessage
         from nanobot.bus.queue import MessageBus
@@ -801,7 +799,6 @@ class TestConsolidationDeduplicationGuard:
         loop = AgentLoop(
             bus=bus, provider=provider, workspace=tmp_path, model="test-model", memory_window=10
         )
-
         loop.provider.chat = AsyncMock(return_value=LLMResponse(content="ok", tool_calls=[]))
         loop.tools.get_definitions = MagicMock(return_value=[])
 
@@ -811,10 +808,6 @@ class TestConsolidationDeduplicationGuard:
             session.add_message("assistant", f"resp{i}")
         loop.sessions.save(session)
 
-        # Ensure lock exists before /new.
-        loop._consolidation_locks.setdefault(session.key, asyncio.Lock())
-        assert session.key in loop._consolidation_locks
-
         async def _ok_consolidate(sess, archive_all: bool = False, **kw) -> bool:
             return True
 
@@ -825,4 +818,4 @@ class TestConsolidationDeduplicationGuard:
 
         assert response is not None
         assert "new session started" in response.content.lower()
-        assert session.key not in loop._consolidation_locks
+        assert loop.sessions.get_or_create("cli:test").messages == []

tests/test_context_prompt_cache.py

@@ -40,7 +40,7 @@ def test_system_prompt_stays_stable_when_clock_changes(tmp_path, monkeypatch) ->
 
 
 def test_runtime_context_is_separate_untrusted_user_message(tmp_path) -> None:
-    """Runtime metadata should be a separate user message before the actual user message."""
+    """Runtime metadata should be merged with the user message."""
     workspace = _make_workspace(tmp_path)
     builder = ContextBuilder(workspace)
 
@@ -54,13 +54,12 @@ def test_runtime_context_is_separate_untrusted_user_message(tmp_path) -> None:
     assert messages[0]["role"] == "system"
     assert "## Current Session" not in messages[0]["content"]
 
-    assert messages[-2]["role"] == "user"
-    runtime_content = messages[-2]["content"]
-    assert isinstance(runtime_content, str)
-    assert ContextBuilder._RUNTIME_CONTEXT_TAG in runtime_content
-    assert "Current Time:" in runtime_content
-    assert "Channel: cli" in runtime_content
-    assert "Chat ID: direct" in runtime_content
-
+    # Runtime context is now merged with user message into a single message
     assert messages[-1]["role"] == "user"
-    assert messages[-1]["content"] == "Return exactly: OK"
+    user_content = messages[-1]["content"]
+    assert isinstance(user_content, str)
+    assert ContextBuilder._RUNTIME_CONTEXT_TAG in user_content
+    assert "Current Time:" in user_content
+    assert "Channel: cli" in user_content
+    assert "Chat ID: direct" in user_content
+    assert "Return exactly: OK" in user_content

tests/test_cron_commands.py

@@ -1,29 +0,0 @@
-from typer.testing import CliRunner
-
-from nanobot.cli.commands import app
-
-runner = CliRunner()
-
-
-def test_cron_add_rejects_invalid_timezone(monkeypatch, tmp_path) -> None:
-    monkeypatch.setattr("nanobot.config.loader.get_data_dir", lambda: tmp_path)
-
-    result = runner.invoke(
-        app,
-        [
-            "cron",
-            "add",
-            "--name",
-            "demo",
-            "--message",
-            "hello",
-            "--cron",
-            "0 9 * * *",
-            "--tz",
-            "America/Vancovuer",
-        ],
-    )
-
-    assert result.exit_code == 1
-    assert "Error: unknown timezone 'America/Vancovuer'" in result.stdout
-    assert not (tmp_path / "cron" / "jobs.json").exists()

tests/test_cron_service.py

@@ -1,3 +1,5 @@
+import asyncio
+
 import pytest
 
 from nanobot.cron.service import CronService
@@ -28,3 +30,32 @@ def test_add_job_accepts_valid_timezone(tmp_path) -> None:
 
     assert job.schedule.tz == "America/Vancouver"
     assert job.state.next_run_at_ms is not None
+
+
+@pytest.mark.asyncio
+async def test_running_service_honors_external_disable(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    called: list[str] = []
+
+    async def on_job(job) -> None:
+        called.append(job.id)
+
+    service = CronService(store_path, on_job=on_job)
+    job = service.add_job(
+        name="external-disable",
+        schedule=CronSchedule(kind="every", every_ms=200),
+        message="hello",
+    )
+    await service.start()
+    try:
+        # Wait slightly to ensure file mtime is definitively different
+        await asyncio.sleep(0.05)
+        external = CronService(store_path)
+        updated = external.enable_job(job.id, enabled=False)
+        assert updated is not None
+        assert updated.enabled is False
+
+        await asyncio.sleep(0.35)
+        assert called == []
+    finally:
+        service.stop()

tests/test_feishu_post_content.py

@@ -0,0 +1,40 @@
+from nanobot.channels.feishu import _extract_post_content
+
+
+def test_extract_post_content_supports_post_wrapper_shape() -> None:
+    payload = {
+        "post": {
+            "zh_cn": {
+                "title": "日报",
+                "content": [
+                    [
+                        {"tag": "text", "text": "完成"},
+                        {"tag": "img", "image_key": "img_1"},
+                    ]
+                ],
+            }
+        }
+    }
+
+    text, image_keys = _extract_post_content(payload)
+
+    assert text == "日报 完成"
+    assert image_keys == ["img_1"]
+
+
+def test_extract_post_content_keeps_direct_shape_behavior() -> None:
+    payload = {
+        "title": "Daily",
+        "content": [
+            [
+                {"tag": "text", "text": "report"},
+                {"tag": "img", "image_key": "img_a"},
+                {"tag": "img", "image_key": "img_b"},
+            ]
+        ],
+    }
+
+    text, image_keys = _extract_post_content(payload)
+
+    assert text == "Daily report"
+    assert image_keys == ["img_a", "img_b"]

tests/test_feishu_table_split.py

@@ -0,0 +1,104 @@
+"""Tests for FeishuChannel._split_elements_by_table_limit.
+
+Feishu cards reject messages that contain more than one table element
+(API error 11310: card table number over limit).  The helper splits a flat
+list of card elements into groups so that each group contains at most one
+table, allowing nanobot to send multiple cards instead of failing.
+"""
+
+from nanobot.channels.feishu import FeishuChannel
+
+
+def _md(text: str) -> dict:
+    return {"tag": "markdown", "content": text}
+
+
+def _table() -> dict:
+    return {
+        "tag": "table",
+        "columns": [{"tag": "column", "name": "c0", "display_name": "A", "width": "auto"}],
+        "rows": [{"c0": "v"}],
+        "page_size": 2,
+    }
+
+
+split = FeishuChannel._split_elements_by_table_limit
+
+
+def test_empty_list_returns_single_empty_group() -> None:
+    assert split([]) == [[]]
+
+
+def test_no_tables_returns_single_group() -> None:
+    els = [_md("hello"), _md("world")]
+    result = split(els)
+    assert result == [els]
+
+
+def test_single_table_stays_in_one_group() -> None:
+    els = [_md("intro"), _table(), _md("outro")]
+    result = split(els)
+    assert len(result) == 1
+    assert result[0] == els
+
+
+def test_two_tables_split_into_two_groups() -> None:
+    # Use different row values so the two tables are not equal
+    t1 = {
+        "tag": "table",
+        "columns": [{"tag": "column", "name": "c0", "display_name": "A", "width": "auto"}],
+        "rows": [{"c0": "table-one"}],
+        "page_size": 2,
+    }
+    t2 = {
+        "tag": "table",
+        "columns": [{"tag": "column", "name": "c0", "display_name": "B", "width": "auto"}],
+        "rows": [{"c0": "table-two"}],
+        "page_size": 2,
+    }
+    els = [_md("before"), t1, _md("between"), t2, _md("after")]
+    result = split(els)
+    assert len(result) == 2
+    # First group: text before table-1 + table-1
+    assert t1 in result[0]
+    assert t2 not in result[0]
+    # Second group: text between tables + table-2 + text after
+    assert t2 in result[1]
+    assert t1 not in result[1]
+
+
+def test_three_tables_split_into_three_groups() -> None:
+    tables = [
+        {"tag": "table", "columns": [], "rows": [{"c0": f"t{i}"}], "page_size": 1}
+        for i in range(3)
+    ]
+    els = tables[:]
+    result = split(els)
+    assert len(result) == 3
+    for i, group in enumerate(result):
+        assert tables[i] in group
+
+
+def test_leading_markdown_stays_with_first_table() -> None:
+    intro = _md("intro")
+    t = _table()
+    result = split([intro, t])
+    assert len(result) == 1
+    assert result[0] == [intro, t]
+
+
+def test_trailing_markdown_after_second_table() -> None:
+    t1, t2 = _table(), _table()
+    tail = _md("end")
+    result = split([t1, t2, tail])
+    assert len(result) == 2
+    assert result[1] == [t2, tail]
+
+
+def test_non_table_elements_before_first_table_kept_in_first_group() -> None:
+    head = _md("head")
+    t1, t2 = _table(), _table()
+    result = split([head, t1, t2])
+    # head + t1 in group 0; t2 in group 1
+    assert result[0] == [head, t1]
+    assert result[1] == [t2]

tests/test_loop_save_turn.py

@@ -0,0 +1,41 @@
+from nanobot.agent.context import ContextBuilder
+from nanobot.agent.loop import AgentLoop
+from nanobot.session.manager import Session
+
+
+def _mk_loop() -> AgentLoop:
+    loop = AgentLoop.__new__(AgentLoop)
+    loop._TOOL_RESULT_MAX_CHARS = 500
+    return loop
+
+
+def test_save_turn_skips_multimodal_user_when_only_runtime_context() -> None:
+    loop = _mk_loop()
+    session = Session(key="test:runtime-only")
+    runtime = ContextBuilder._RUNTIME_CONTEXT_TAG + "\nCurrent Time: now (UTC)"
+
+    loop._save_turn(
+        session,
+        [{"role": "user", "content": [{"type": "text", "text": runtime}]}],
+        skip=0,
+    )
+    assert session.messages == []
+
+
+def test_save_turn_keeps_image_placeholder_after_runtime_strip() -> None:
+    loop = _mk_loop()
+    session = Session(key="test:image")
+    runtime = ContextBuilder._RUNTIME_CONTEXT_TAG + "\nCurrent Time: now (UTC)"
+
+    loop._save_turn(
+        session,
+        [{
+            "role": "user",
+            "content": [
+                {"type": "text", "text": runtime},
+                {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}},
+            ],
+        }],
+        skip=0,
+    )
+    assert session.messages[0]["content"] == [{"type": "text", "text": "[image]"}]

tests/test_matrix_channel.py

@@ -159,6 +159,7 @@ class _FakeAsyncClient:
 
 
 def _make_config(**kwargs) -> MatrixConfig:
+    kwargs.setdefault("allow_from", ["*"])
     return MatrixConfig(
         enabled=True,
         homeserver="https://matrix.org",
@@ -274,7 +275,7 @@ async def test_stop_stops_sync_forever_before_close(monkeypatch) -> None:
 
 
 @pytest.mark.asyncio
-async def test_room_invite_joins_when_allow_list_is_empty() -> None:
+async def test_room_invite_ignores_when_allow_list_is_empty() -> None:
     channel = MatrixChannel(_make_config(allow_from=[]), MessageBus())
     client = _FakeAsyncClient("", "", "", None)
     channel.client = client
@@ -284,9 +285,22 @@ async def test_room_invite_joins_when_allow_list_is_empty() -> None:
 
     await channel._on_room_invite(room, event)
 
-    assert client.join_calls == ["!room:matrix.org"]
+    assert client.join_calls == []
 
 
+@pytest.mark.asyncio
+async def test_room_invite_joins_when_sender_allowed() -> None:
+    channel = MatrixChannel(_make_config(allow_from=["@alice:matrix.org"]), MessageBus())
+    client = _FakeAsyncClient("", "", "", None)
+    channel.client = client
+
+    room = SimpleNamespace(room_id="!room:matrix.org")
+    event = SimpleNamespace(sender="@alice:matrix.org")
+
+    await channel._on_room_invite(room, event)
+
+    assert client.join_calls == ["!room:matrix.org"]
+
 @pytest.mark.asyncio
 async def test_room_invite_respects_allow_list_when_configured() -> None:
     channel = MatrixChannel(_make_config(allow_from=["@bob:matrix.org"]), MessageBus())
@@ -1163,6 +1177,8 @@ async def test_send_progress_keeps_typing_keepalive_running() -> None:
     assert "!room:matrix.org" in channel._typing_tasks
     assert client.typing_calls[-1] == ("!room:matrix.org", True, TYPING_NOTICE_TIMEOUT_MS)
 
+    await channel.stop()
+
 
 @pytest.mark.asyncio
 async def test_send_clears_typing_when_send_fails() -> None:

tests/test_memory_consolidation_types.py

@@ -145,3 +145,78 @@ class TestMemoryConsolidationTypeHandling:
 
         assert result is True
         provider.chat.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_list_arguments_extracts_first_dict(self, tmp_path: Path) -> None:
+        """Some providers return arguments as a list - extract first element if it's a dict."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+
+        # Simulate arguments being a list containing a dict
+        response = LLMResponse(
+            content=None,
+            tool_calls=[
+                ToolCallRequest(
+                    id="call_1",
+                    name="save_memory",
+                    arguments=[{
+                        "history_entry": "[2026-01-01] User discussed testing.",
+                        "memory_update": "# Memory\nUser likes testing.",
+                    }],
+                )
+            ],
+        )
+        provider.chat = AsyncMock(return_value=response)
+        session = _make_session(message_count=60)
+
+        result = await store.consolidate(session, provider, "test-model", memory_window=50)
+
+        assert result is True
+        assert "User discussed testing." in store.history_file.read_text()
+        assert "User likes testing." in store.memory_file.read_text()
+
+    @pytest.mark.asyncio
+    async def test_list_arguments_empty_list_returns_false(self, tmp_path: Path) -> None:
+        """Empty list arguments should return False."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+
+        response = LLMResponse(
+            content=None,
+            tool_calls=[
+                ToolCallRequest(
+                    id="call_1",
+                    name="save_memory",
+                    arguments=[],
+                )
+            ],
+        )
+        provider.chat = AsyncMock(return_value=response)
+        session = _make_session(message_count=60)
+
+        result = await store.consolidate(session, provider, "test-model", memory_window=50)
+
+        assert result is False
+
+    @pytest.mark.asyncio
+    async def test_list_arguments_non_dict_content_returns_false(self, tmp_path: Path) -> None:
+        """List with non-dict content should return False."""
+        store = MemoryStore(tmp_path)
+        provider = AsyncMock()
+
+        response = LLMResponse(
+            content=None,
+            tool_calls=[
+                ToolCallRequest(
+                    id="call_1",
+                    name="save_memory",
+                    arguments=["string", "content"],
+                )
+            ],
+        )
+        provider.chat = AsyncMock(return_value=response)
+        session = _make_session(message_count=60)
+
+        result = await store.consolidate(session, provider, "test-model", memory_window=50)
+
+        assert result is False