Merge origin/main into fix/sanitize-messages-non-claude
Resolved conflict in azure_openai_provider.py by keeping main's Responses API implementation (role alternation not needed for the Responses API input format). Made-with: Cursor
This commit is contained in:
@@ -506,7 +506,7 @@ class TestNewCommandArchival:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
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."""
|
||||
"""/new clears session immediately; archive is fire-and-forget."""
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop = self._make_loop(tmp_path)
|
||||
@@ -518,12 +518,12 @@ class TestNewCommandArchival:
|
||||
|
||||
call_count = 0
|
||||
|
||||
async def _failing_consolidate(_messages) -> bool:
|
||||
async def _failing_summarize(_messages) -> bool:
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
return False
|
||||
|
||||
loop.memory_consolidator.consolidate_messages = _failing_consolidate # type: ignore[method-assign]
|
||||
loop.consolidator.archive = _failing_summarize # type: ignore[method-assign]
|
||||
|
||||
new_msg = InboundMessage(channel="cli", sender_id="user", chat_id="test", content="/new")
|
||||
response = await loop._process_message(new_msg)
|
||||
@@ -535,7 +535,7 @@ class TestNewCommandArchival:
|
||||
assert len(session_after.messages) == 0
|
||||
|
||||
await loop.close_mcp()
|
||||
assert call_count == 3 # retried up to raw-archive threshold
|
||||
assert call_count == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_new_archives_only_unconsolidated_messages(self, tmp_path: Path) -> None:
|
||||
@@ -551,12 +551,12 @@ class TestNewCommandArchival:
|
||||
|
||||
archived_count = -1
|
||||
|
||||
async def _fake_consolidate(messages) -> bool:
|
||||
async def _fake_summarize(messages) -> bool:
|
||||
nonlocal archived_count
|
||||
archived_count = len(messages)
|
||||
return True
|
||||
|
||||
loop.memory_consolidator.consolidate_messages = _fake_consolidate # type: ignore[method-assign]
|
||||
loop.consolidator.archive = _fake_summarize # type: ignore[method-assign]
|
||||
|
||||
new_msg = InboundMessage(channel="cli", sender_id="user", chat_id="test", content="/new")
|
||||
response = await loop._process_message(new_msg)
|
||||
@@ -578,10 +578,10 @@ class TestNewCommandArchival:
|
||||
session.add_message("assistant", f"resp{i}")
|
||||
loop.sessions.save(session)
|
||||
|
||||
async def _ok_consolidate(_messages) -> bool:
|
||||
async def _ok_summarize(_messages) -> bool:
|
||||
return True
|
||||
|
||||
loop.memory_consolidator.consolidate_messages = _ok_consolidate # type: ignore[method-assign]
|
||||
loop.consolidator.archive = _ok_summarize # type: ignore[method-assign]
|
||||
|
||||
new_msg = InboundMessage(channel="cli", sender_id="user", chat_id="test", content="/new")
|
||||
response = await loop._process_message(new_msg)
|
||||
@@ -604,12 +604,12 @@ class TestNewCommandArchival:
|
||||
|
||||
archived = asyncio.Event()
|
||||
|
||||
async def _slow_consolidate(_messages) -> bool:
|
||||
async def _slow_summarize(_messages) -> bool:
|
||||
await asyncio.sleep(0.1)
|
||||
archived.set()
|
||||
return True
|
||||
|
||||
loop.memory_consolidator.consolidate_messages = _slow_consolidate # type: ignore[method-assign]
|
||||
loop.consolidator.archive = _slow_summarize # type: ignore[method-assign]
|
||||
|
||||
new_msg = InboundMessage(channel="cli", sender_id="user", chat_id="test", content="/new")
|
||||
await loop._process_message(new_msg)
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
"""Tests for the lightweight Consolidator — append-only to HISTORY.md."""
|
||||
|
||||
import pytest
|
||||
import asyncio
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
from nanobot.agent.memory import Consolidator, MemoryStore
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store(tmp_path):
|
||||
return MemoryStore(tmp_path)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_provider():
|
||||
p = MagicMock()
|
||||
p.chat_with_retry = AsyncMock()
|
||||
return p
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def consolidator(store, mock_provider):
|
||||
sessions = MagicMock()
|
||||
sessions.save = MagicMock()
|
||||
return Consolidator(
|
||||
store=store,
|
||||
provider=mock_provider,
|
||||
model="test-model",
|
||||
sessions=sessions,
|
||||
context_window_tokens=1000,
|
||||
build_messages=MagicMock(return_value=[]),
|
||||
get_tool_definitions=MagicMock(return_value=[]),
|
||||
max_completion_tokens=100,
|
||||
)
|
||||
|
||||
|
||||
class TestConsolidatorSummarize:
|
||||
async def test_summarize_appends_to_history(self, consolidator, mock_provider, store):
|
||||
"""Consolidator should call LLM to summarize, then append to HISTORY.md."""
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(
|
||||
content="User fixed a bug in the auth module."
|
||||
)
|
||||
messages = [
|
||||
{"role": "user", "content": "fix the auth bug"},
|
||||
{"role": "assistant", "content": "Done, fixed the race condition."},
|
||||
]
|
||||
result = await consolidator.archive(messages)
|
||||
assert result is True
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
|
||||
async def test_summarize_raw_dumps_on_llm_failure(self, consolidator, mock_provider, store):
|
||||
"""On LLM failure, raw-dump messages to HISTORY.md."""
|
||||
mock_provider.chat_with_retry.side_effect = Exception("API error")
|
||||
messages = [{"role": "user", "content": "hello"}]
|
||||
result = await consolidator.archive(messages)
|
||||
assert result is True # always succeeds
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert "[RAW]" in entries[0]["content"]
|
||||
|
||||
async def test_summarize_skips_empty_messages(self, consolidator):
|
||||
result = await consolidator.archive([])
|
||||
assert result is False
|
||||
|
||||
|
||||
class TestConsolidatorTokenBudget:
|
||||
async def test_prompt_below_threshold_does_not_consolidate(self, consolidator):
|
||||
"""No consolidation when tokens are within budget."""
|
||||
session = MagicMock()
|
||||
session.last_consolidated = 0
|
||||
session.messages = [{"role": "user", "content": "hi"}]
|
||||
session.key = "test:key"
|
||||
consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(100, "tiktoken"))
|
||||
consolidator.archive = AsyncMock(return_value=True)
|
||||
await consolidator.maybe_consolidate_by_tokens(session)
|
||||
consolidator.archive.assert_not_called()
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from datetime import datetime as real_datetime
|
||||
from importlib.resources import files as pkg_files
|
||||
from pathlib import Path
|
||||
@@ -47,6 +48,19 @@ def test_system_prompt_stays_stable_when_clock_changes(tmp_path, monkeypatch) ->
|
||||
assert prompt1 == prompt2
|
||||
|
||||
|
||||
def test_system_prompt_reflects_current_dream_memory_contract(tmp_path) -> None:
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
|
||||
assert "memory/history.jsonl" in prompt
|
||||
assert "automatically managed by Dream" in prompt
|
||||
assert "do not edit directly" in prompt
|
||||
assert "memory/HISTORY.md" not in prompt
|
||||
assert "write important facts here" not in prompt
|
||||
|
||||
|
||||
def test_runtime_context_is_separate_untrusted_user_message(tmp_path) -> None:
|
||||
"""Runtime metadata should be merged with the user message."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
@@ -71,3 +85,137 @@ def test_runtime_context_is_separate_untrusted_user_message(tmp_path) -> None:
|
||||
assert "Channel: cli" in user_content
|
||||
assert "Chat ID: direct" in user_content
|
||||
assert "Return exactly: OK" in user_content
|
||||
|
||||
|
||||
def test_unprocessed_history_injected_into_system_prompt(tmp_path) -> None:
|
||||
"""Entries in history.jsonl not yet consumed by Dream appear with timestamps."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
builder.memory.append_history("User asked about weather in Tokyo")
|
||||
builder.memory.append_history("Agent fetched forecast via web_search")
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
assert "# Recent History" in prompt
|
||||
assert "User asked about weather in Tokyo" in prompt
|
||||
assert "Agent fetched forecast via web_search" in prompt
|
||||
assert re.search(r"\[\d{4}-\d{2}-\d{2} \d{2}:\d{2}\]", prompt)
|
||||
|
||||
|
||||
def test_recent_history_capped_at_max(tmp_path) -> None:
|
||||
"""Only the most recent _MAX_RECENT_HISTORY entries are injected."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
for i in range(builder._MAX_RECENT_HISTORY + 20):
|
||||
builder.memory.append_history(f"entry-{i}")
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
assert "entry-0" not in prompt
|
||||
assert "entry-19" not in prompt
|
||||
assert f"entry-{builder._MAX_RECENT_HISTORY + 19}" in prompt
|
||||
|
||||
|
||||
def test_no_recent_history_when_dream_has_processed_all(tmp_path) -> None:
|
||||
"""If Dream has consumed everything, no Recent History section should appear."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
cursor = builder.memory.append_history("already processed entry")
|
||||
builder.memory.set_last_dream_cursor(cursor)
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
assert "# Recent History" not in prompt
|
||||
|
||||
|
||||
def test_partial_dream_processing_shows_only_remainder(tmp_path) -> None:
|
||||
"""When Dream has processed some entries, only the unprocessed ones appear."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
c1 = builder.memory.append_history("old conversation about Python")
|
||||
c2 = builder.memory.append_history("old conversation about Rust")
|
||||
builder.memory.append_history("recent question about Docker")
|
||||
builder.memory.append_history("recent question about K8s")
|
||||
|
||||
builder.memory.set_last_dream_cursor(c2)
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
assert "# Recent History" in prompt
|
||||
assert "old conversation about Python" not in prompt
|
||||
assert "old conversation about Rust" not in prompt
|
||||
assert "recent question about Docker" in prompt
|
||||
assert "recent question about K8s" in prompt
|
||||
|
||||
|
||||
def test_execution_rules_in_system_prompt(tmp_path) -> None:
|
||||
"""New execution rules should appear in the system prompt."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
prompt = builder.build_system_prompt()
|
||||
assert "Act, don't narrate" in prompt
|
||||
assert "Read before you write" in prompt
|
||||
assert "verify the result" in prompt
|
||||
|
||||
|
||||
def test_channel_format_hint_telegram(tmp_path) -> None:
|
||||
"""Telegram channel should get messaging-app format hint."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
prompt = builder.build_system_prompt(channel="telegram")
|
||||
assert "Format Hint" in prompt
|
||||
assert "messaging app" in prompt
|
||||
|
||||
|
||||
def test_channel_format_hint_whatsapp(tmp_path) -> None:
|
||||
"""WhatsApp should get plain-text format hint."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
prompt = builder.build_system_prompt(channel="whatsapp")
|
||||
assert "Format Hint" in prompt
|
||||
assert "plain text only" in prompt
|
||||
|
||||
|
||||
def test_channel_format_hint_absent_for_unknown(tmp_path) -> None:
|
||||
"""Unknown or None channel should not inject a format hint."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
prompt = builder.build_system_prompt(channel=None)
|
||||
assert "Format Hint" not in prompt
|
||||
|
||||
prompt2 = builder.build_system_prompt(channel="feishu")
|
||||
assert "Format Hint" not in prompt2
|
||||
|
||||
|
||||
def test_build_messages_passes_channel_to_system_prompt(tmp_path) -> None:
|
||||
"""build_messages should pass channel through to build_system_prompt."""
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
messages = builder.build_messages(
|
||||
history=[], current_message="hi",
|
||||
channel="telegram", chat_id="123",
|
||||
)
|
||||
system = messages[0]["content"]
|
||||
assert "Format Hint" in system
|
||||
assert "messaging app" in system
|
||||
|
||||
|
||||
def test_subagent_result_does_not_create_consecutive_assistant_messages(tmp_path) -> None:
|
||||
workspace = _make_workspace(tmp_path)
|
||||
builder = ContextBuilder(workspace)
|
||||
|
||||
messages = builder.build_messages(
|
||||
history=[{"role": "assistant", "content": "previous result"}],
|
||||
current_message="subagent result",
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
current_role="assistant",
|
||||
)
|
||||
|
||||
for left, right in zip(messages, messages[1:]):
|
||||
assert not (left.get("role") == right.get("role") == "assistant")
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
"""Tests for the Dream class — two-phase memory consolidation via AgentRunner."""
|
||||
|
||||
import pytest
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from nanobot.agent.memory import Dream, MemoryStore
|
||||
from nanobot.agent.runner import AgentRunResult
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store(tmp_path):
|
||||
s = MemoryStore(tmp_path)
|
||||
s.write_soul("# Soul\n- Helpful")
|
||||
s.write_user("# User\n- Developer")
|
||||
s.write_memory("# Memory\n- Project X active")
|
||||
return s
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_provider():
|
||||
p = MagicMock()
|
||||
p.chat_with_retry = AsyncMock()
|
||||
return p
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_runner():
|
||||
return MagicMock()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def dream(store, mock_provider, mock_runner):
|
||||
d = Dream(store=store, provider=mock_provider, model="test-model", max_batch_size=5)
|
||||
d._runner = mock_runner
|
||||
return d
|
||||
|
||||
|
||||
def _make_run_result(
|
||||
stop_reason="completed",
|
||||
final_content=None,
|
||||
tool_events=None,
|
||||
usage=None,
|
||||
):
|
||||
return AgentRunResult(
|
||||
final_content=final_content or stop_reason,
|
||||
stop_reason=stop_reason,
|
||||
messages=[],
|
||||
tools_used=[],
|
||||
usage={},
|
||||
tool_events=tool_events or [],
|
||||
)
|
||||
|
||||
|
||||
class TestDreamRun:
|
||||
async def test_noop_when_no_unprocessed_history(self, dream, mock_provider, mock_runner, store):
|
||||
"""Dream should not call LLM when there's nothing to process."""
|
||||
result = await dream.run()
|
||||
assert result is False
|
||||
mock_provider.chat_with_retry.assert_not_called()
|
||||
mock_runner.run.assert_not_called()
|
||||
|
||||
async def test_calls_runner_for_unprocessed_entries(self, dream, mock_provider, mock_runner, store):
|
||||
"""Dream should call AgentRunner when there are unprocessed history entries."""
|
||||
store.append_history("User prefers dark mode")
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(content="New fact")
|
||||
mock_runner.run = AsyncMock(return_value=_make_run_result(
|
||||
tool_events=[{"name": "edit_file", "status": "ok", "detail": "memory/MEMORY.md"}],
|
||||
))
|
||||
result = await dream.run()
|
||||
assert result is True
|
||||
mock_runner.run.assert_called_once()
|
||||
spec = mock_runner.run.call_args[0][0]
|
||||
assert spec.max_iterations == 10
|
||||
assert spec.fail_on_tool_error is False
|
||||
|
||||
async def test_advances_dream_cursor(self, dream, mock_provider, mock_runner, store):
|
||||
"""Dream should advance the cursor after processing."""
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(content="Nothing new")
|
||||
mock_runner.run = AsyncMock(return_value=_make_run_result())
|
||||
await dream.run()
|
||||
assert store.get_last_dream_cursor() == 2
|
||||
|
||||
async def test_compacts_processed_history(self, dream, mock_provider, mock_runner, store):
|
||||
"""Dream should compact history after processing."""
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
store.append_history("event 3")
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(content="Nothing new")
|
||||
mock_runner.run = AsyncMock(return_value=_make_run_result())
|
||||
await dream.run()
|
||||
# After Dream, cursor is advanced and 3, compact keeps last max_history_entries
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert all(e["cursor"] > 0 for e in entries)
|
||||
|
||||
@@ -184,17 +184,22 @@ def test_stale_extra_content_in_tool_calls_survives_sanitize() -> None:
|
||||
with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"):
|
||||
provider = OpenAICompatProvider()
|
||||
|
||||
messages = [{
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [{
|
||||
"id": "call_1",
|
||||
"type": "function",
|
||||
"function": {"name": "fn", "arguments": "{}"},
|
||||
"extra_content": GEMINI_EXTRA,
|
||||
}],
|
||||
}]
|
||||
messages = [
|
||||
{"role": "user", "content": "hi"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [{
|
||||
"id": "call_1",
|
||||
"type": "function",
|
||||
"function": {"name": "fn", "arguments": "{}"},
|
||||
"extra_content": GEMINI_EXTRA,
|
||||
}],
|
||||
},
|
||||
{"role": "tool", "content": "ok", "tool_call_id": "call_1"},
|
||||
{"role": "user", "content": "thanks"},
|
||||
]
|
||||
|
||||
sanitized = provider._sanitize_messages(messages)
|
||||
|
||||
assert sanitized[0]["tool_calls"][0]["extra_content"] == GEMINI_EXTRA
|
||||
assert sanitized[1]["tool_calls"][0]["extra_content"] == GEMINI_EXTRA
|
||||
|
||||
@@ -0,0 +1,234 @@
|
||||
"""Tests for GitStore — git-backed version control for memory files."""
|
||||
|
||||
import pytest
|
||||
from pathlib import Path
|
||||
|
||||
from nanobot.utils.gitstore import GitStore, CommitInfo
|
||||
|
||||
|
||||
TRACKED = ["SOUL.md", "USER.md", "memory/MEMORY.md"]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def git(tmp_path):
|
||||
"""Uninitialized GitStore."""
|
||||
return GitStore(tmp_path, tracked_files=TRACKED)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def git_ready(git):
|
||||
"""Initialized GitStore with one initial commit."""
|
||||
git.init()
|
||||
return git
|
||||
|
||||
|
||||
class TestInit:
|
||||
def test_not_initialized_by_default(self, git, tmp_path):
|
||||
assert not git.is_initialized()
|
||||
assert not (tmp_path / ".git").is_dir()
|
||||
|
||||
def test_init_creates_git_dir(self, git, tmp_path):
|
||||
assert git.init()
|
||||
assert (tmp_path / ".git").is_dir()
|
||||
|
||||
def test_init_idempotent(self, git_ready):
|
||||
assert not git_ready.init()
|
||||
|
||||
def test_init_creates_gitignore(self, git_ready):
|
||||
gi = git_ready._workspace / ".gitignore"
|
||||
assert gi.exists()
|
||||
content = gi.read_text(encoding="utf-8")
|
||||
for f in TRACKED:
|
||||
assert f"!{f}" in content
|
||||
|
||||
def test_init_touches_tracked_files(self, git_ready):
|
||||
for f in TRACKED:
|
||||
assert (git_ready._workspace / f).exists()
|
||||
|
||||
def test_init_makes_initial_commit(self, git_ready):
|
||||
commits = git_ready.log()
|
||||
assert len(commits) == 1
|
||||
assert "init" in commits[0].message
|
||||
|
||||
|
||||
class TestBuildGitignore:
|
||||
def test_subdirectory_dirs(self, git):
|
||||
content = git._build_gitignore()
|
||||
assert "!memory/\n" in content
|
||||
for f in TRACKED:
|
||||
assert f"!{f}\n" in content
|
||||
assert content.startswith("/*\n")
|
||||
|
||||
def test_root_level_files_no_dir_entries(self, tmp_path):
|
||||
gs = GitStore(tmp_path, tracked_files=["a.md", "b.md"])
|
||||
content = gs._build_gitignore()
|
||||
assert "!a.md\n" in content
|
||||
assert "!b.md\n" in content
|
||||
dir_lines = [l for l in content.split("\n") if l.startswith("!") and l.endswith("/")]
|
||||
assert dir_lines == []
|
||||
|
||||
|
||||
class TestAutoCommit:
|
||||
def test_returns_none_when_not_initialized(self, git):
|
||||
assert git.auto_commit("test") is None
|
||||
|
||||
def test_commits_file_change(self, git_ready):
|
||||
(git_ready._workspace / "SOUL.md").write_text("updated", encoding="utf-8")
|
||||
sha = git_ready.auto_commit("update soul")
|
||||
assert sha is not None
|
||||
assert len(sha) == 8
|
||||
|
||||
def test_returns_none_when_no_changes(self, git_ready):
|
||||
assert git_ready.auto_commit("no change") is None
|
||||
|
||||
def test_commit_appears_in_log(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("v2", encoding="utf-8")
|
||||
sha = git_ready.auto_commit("update soul")
|
||||
commits = git_ready.log()
|
||||
assert len(commits) == 2
|
||||
assert commits[0].sha == sha
|
||||
|
||||
def test_does_not_create_empty_commits(self, git_ready):
|
||||
git_ready.auto_commit("nothing 1")
|
||||
git_ready.auto_commit("nothing 2")
|
||||
assert len(git_ready.log()) == 1 # only init commit
|
||||
|
||||
|
||||
class TestLog:
|
||||
def test_empty_when_not_initialized(self, git):
|
||||
assert git.log() == []
|
||||
|
||||
def test_newest_first(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
for i in range(3):
|
||||
(ws / "SOUL.md").write_text(f"v{i}", encoding="utf-8")
|
||||
git_ready.auto_commit(f"commit {i}")
|
||||
|
||||
commits = git_ready.log()
|
||||
assert len(commits) == 4 # init + 3
|
||||
assert "commit 2" in commits[0].message
|
||||
assert "init" in commits[-1].message
|
||||
|
||||
def test_max_entries(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
for i in range(10):
|
||||
(ws / "SOUL.md").write_text(f"v{i}", encoding="utf-8")
|
||||
git_ready.auto_commit(f"c{i}")
|
||||
assert len(git_ready.log(max_entries=3)) == 3
|
||||
|
||||
def test_commit_info_fields(self, git_ready):
|
||||
c = git_ready.log()[0]
|
||||
assert isinstance(c, CommitInfo)
|
||||
assert len(c.sha) == 8
|
||||
assert c.timestamp
|
||||
assert c.message
|
||||
|
||||
|
||||
class TestDiffCommits:
|
||||
def test_empty_when_not_initialized(self, git):
|
||||
assert git.diff_commits("a", "b") == ""
|
||||
|
||||
def test_diff_between_two_commits(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("original", encoding="utf-8")
|
||||
git_ready.auto_commit("v1")
|
||||
(ws / "SOUL.md").write_text("modified", encoding="utf-8")
|
||||
git_ready.auto_commit("v2")
|
||||
|
||||
commits = git_ready.log()
|
||||
diff = git_ready.diff_commits(commits[1].sha, commits[0].sha)
|
||||
assert "modified" in diff
|
||||
|
||||
def test_invalid_sha_returns_empty(self, git_ready):
|
||||
assert git_ready.diff_commits("deadbeef", "cafebabe") == ""
|
||||
|
||||
|
||||
class TestFindCommit:
|
||||
def test_finds_by_prefix(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("v2", encoding="utf-8")
|
||||
sha = git_ready.auto_commit("v2")
|
||||
found = git_ready.find_commit(sha[:4])
|
||||
assert found is not None
|
||||
assert found.sha == sha
|
||||
|
||||
def test_returns_none_for_unknown(self, git_ready):
|
||||
assert git_ready.find_commit("deadbeef") is None
|
||||
|
||||
|
||||
class TestShowCommitDiff:
|
||||
def test_returns_commit_with_diff(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("content", encoding="utf-8")
|
||||
sha = git_ready.auto_commit("add content")
|
||||
result = git_ready.show_commit_diff(sha)
|
||||
assert result is not None
|
||||
commit, diff = result
|
||||
assert commit.sha == sha
|
||||
assert "content" in diff
|
||||
|
||||
def test_first_commit_has_empty_diff(self, git_ready):
|
||||
init_sha = git_ready.log()[-1].sha
|
||||
result = git_ready.show_commit_diff(init_sha)
|
||||
assert result is not None
|
||||
_, diff = result
|
||||
assert diff == ""
|
||||
|
||||
def test_returns_none_for_unknown(self, git_ready):
|
||||
assert git_ready.show_commit_diff("deadbeef") is None
|
||||
|
||||
|
||||
class TestCommitInfoFormat:
|
||||
def test_format_with_diff(self):
|
||||
from nanobot.utils.gitstore import CommitInfo
|
||||
c = CommitInfo(sha="abcd1234", message="test commit\nsecond line", timestamp="2026-04-02 12:00")
|
||||
result = c.format(diff="some diff")
|
||||
assert "test commit" in result
|
||||
assert "`abcd1234`" in result
|
||||
assert "some diff" in result
|
||||
|
||||
def test_format_without_diff(self):
|
||||
from nanobot.utils.gitstore import CommitInfo
|
||||
c = CommitInfo(sha="abcd1234", message="test", timestamp="2026-04-02 12:00")
|
||||
result = c.format()
|
||||
assert "(no file changes)" in result
|
||||
|
||||
|
||||
class TestRevert:
|
||||
def test_returns_none_when_not_initialized(self, git):
|
||||
assert git.revert("abc") is None
|
||||
|
||||
def test_undoes_commit_changes(self, git_ready):
|
||||
"""revert(sha) should undo the given commit by restoring to its parent."""
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("v2 content", encoding="utf-8")
|
||||
git_ready.auto_commit("v2")
|
||||
|
||||
commits = git_ready.log()
|
||||
# commits[0] = v2 (HEAD), commits[1] = init
|
||||
# Revert v2 → restore to init's state (empty SOUL.md)
|
||||
new_sha = git_ready.revert(commits[0].sha)
|
||||
assert new_sha is not None
|
||||
assert (ws / "SOUL.md").read_text(encoding="utf-8") == ""
|
||||
|
||||
def test_root_commit_returns_none(self, git_ready):
|
||||
"""Cannot revert the root commit (no parent to restore to)."""
|
||||
commits = git_ready.log()
|
||||
assert len(commits) == 1
|
||||
assert git_ready.revert(commits[0].sha) is None
|
||||
|
||||
def test_invalid_sha_returns_none(self, git_ready):
|
||||
assert git_ready.revert("deadbeef") is None
|
||||
|
||||
|
||||
class TestMemoryStoreGitProperty:
|
||||
def test_git_property_exposes_gitstore(self, tmp_path):
|
||||
from nanobot.agent.memory import MemoryStore
|
||||
store = MemoryStore(tmp_path)
|
||||
assert isinstance(store.git, GitStore)
|
||||
|
||||
def test_git_property_is_same_object(self, tmp_path):
|
||||
from nanobot.agent.memory import MemoryStore
|
||||
store = MemoryStore(tmp_path)
|
||||
assert store.git is store._git
|
||||
@@ -0,0 +1,381 @@
|
||||
"""Tests for CompositeHook fan-out, error isolation, and integration."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext, CompositeHook
|
||||
|
||||
|
||||
def _ctx() -> AgentHookContext:
|
||||
return AgentHookContext(iteration=0, messages=[])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fan-out: every hook is called in order
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_fans_out_before_iteration():
|
||||
calls: list[str] = []
|
||||
|
||||
class H(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
calls.append(f"A:{context.iteration}")
|
||||
|
||||
class H2(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
calls.append(f"B:{context.iteration}")
|
||||
|
||||
hook = CompositeHook([H(), H2()])
|
||||
ctx = _ctx()
|
||||
await hook.before_iteration(ctx)
|
||||
assert calls == ["A:0", "B:0"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_fans_out_all_async_methods():
|
||||
"""Verify all async methods fan out to every hook."""
|
||||
events: list[str] = []
|
||||
|
||||
class RecordingHook(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
events.append("before_iteration")
|
||||
|
||||
async def on_stream(self, context: AgentHookContext, delta: str) -> None:
|
||||
events.append(f"on_stream:{delta}")
|
||||
|
||||
async def on_stream_end(self, context: AgentHookContext, *, resuming: bool) -> None:
|
||||
events.append(f"on_stream_end:{resuming}")
|
||||
|
||||
async def before_execute_tools(self, context: AgentHookContext) -> None:
|
||||
events.append("before_execute_tools")
|
||||
|
||||
async def after_iteration(self, context: AgentHookContext) -> None:
|
||||
events.append("after_iteration")
|
||||
|
||||
hook = CompositeHook([RecordingHook(), RecordingHook()])
|
||||
ctx = _ctx()
|
||||
|
||||
await hook.before_iteration(ctx)
|
||||
await hook.on_stream(ctx, "hi")
|
||||
await hook.on_stream_end(ctx, resuming=True)
|
||||
await hook.before_execute_tools(ctx)
|
||||
await hook.after_iteration(ctx)
|
||||
|
||||
assert events == [
|
||||
"before_iteration", "before_iteration",
|
||||
"on_stream:hi", "on_stream:hi",
|
||||
"on_stream_end:True", "on_stream_end:True",
|
||||
"before_execute_tools", "before_execute_tools",
|
||||
"after_iteration", "after_iteration",
|
||||
]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Error isolation: one hook raises, others still run
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_error_isolation_before_iteration():
|
||||
calls: list[str] = []
|
||||
|
||||
class Bad(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
class Good(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
calls.append("good")
|
||||
|
||||
hook = CompositeHook([Bad(), Good()])
|
||||
await hook.before_iteration(_ctx())
|
||||
assert calls == ["good"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_error_isolation_on_stream():
|
||||
calls: list[str] = []
|
||||
|
||||
class Bad(AgentHook):
|
||||
async def on_stream(self, context: AgentHookContext, delta: str) -> None:
|
||||
raise RuntimeError("stream-boom")
|
||||
|
||||
class Good(AgentHook):
|
||||
async def on_stream(self, context: AgentHookContext, delta: str) -> None:
|
||||
calls.append(delta)
|
||||
|
||||
hook = CompositeHook([Bad(), Good()])
|
||||
await hook.on_stream(_ctx(), "delta")
|
||||
assert calls == ["delta"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_error_isolation_all_async():
|
||||
"""Error isolation for on_stream_end, before_execute_tools, after_iteration."""
|
||||
calls: list[str] = []
|
||||
|
||||
class Bad(AgentHook):
|
||||
async def on_stream_end(self, context, *, resuming):
|
||||
raise RuntimeError("err")
|
||||
async def before_execute_tools(self, context):
|
||||
raise RuntimeError("err")
|
||||
async def after_iteration(self, context):
|
||||
raise RuntimeError("err")
|
||||
|
||||
class Good(AgentHook):
|
||||
async def on_stream_end(self, context, *, resuming):
|
||||
calls.append("on_stream_end")
|
||||
async def before_execute_tools(self, context):
|
||||
calls.append("before_execute_tools")
|
||||
async def after_iteration(self, context):
|
||||
calls.append("after_iteration")
|
||||
|
||||
hook = CompositeHook([Bad(), Good()])
|
||||
ctx = _ctx()
|
||||
await hook.on_stream_end(ctx, resuming=False)
|
||||
await hook.before_execute_tools(ctx)
|
||||
await hook.after_iteration(ctx)
|
||||
assert calls == ["on_stream_end", "before_execute_tools", "after_iteration"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# finalize_content: pipeline semantics (no error isolation)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_composite_finalize_content_pipeline():
|
||||
class Upper(AgentHook):
|
||||
def finalize_content(self, context, content):
|
||||
return content.upper() if content else content
|
||||
|
||||
class Suffix(AgentHook):
|
||||
def finalize_content(self, context, content):
|
||||
return (content + "!") if content else content
|
||||
|
||||
hook = CompositeHook([Upper(), Suffix()])
|
||||
result = hook.finalize_content(_ctx(), "hello")
|
||||
assert result == "HELLO!"
|
||||
|
||||
|
||||
def test_composite_finalize_content_none_passthrough():
|
||||
hook = CompositeHook([AgentHook()])
|
||||
assert hook.finalize_content(_ctx(), None) is None
|
||||
|
||||
|
||||
def test_composite_finalize_content_ordering():
|
||||
"""First hook transforms first, result feeds second hook."""
|
||||
steps: list[str] = []
|
||||
|
||||
class H1(AgentHook):
|
||||
def finalize_content(self, context, content):
|
||||
steps.append(f"H1:{content}")
|
||||
return content.upper()
|
||||
|
||||
class H2(AgentHook):
|
||||
def finalize_content(self, context, content):
|
||||
steps.append(f"H2:{content}")
|
||||
return content + "!"
|
||||
|
||||
hook = CompositeHook([H1(), H2()])
|
||||
result = hook.finalize_content(_ctx(), "hi")
|
||||
assert result == "HI!"
|
||||
assert steps == ["H1:hi", "H2:HI"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# wants_streaming: any-semantics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_composite_wants_streaming_any_true():
|
||||
class No(AgentHook):
|
||||
def wants_streaming(self):
|
||||
return False
|
||||
|
||||
class Yes(AgentHook):
|
||||
def wants_streaming(self):
|
||||
return True
|
||||
|
||||
hook = CompositeHook([No(), Yes(), No()])
|
||||
assert hook.wants_streaming() is True
|
||||
|
||||
|
||||
def test_composite_wants_streaming_all_false():
|
||||
hook = CompositeHook([AgentHook(), AgentHook()])
|
||||
assert hook.wants_streaming() is False
|
||||
|
||||
|
||||
def test_composite_wants_streaming_empty():
|
||||
hook = CompositeHook([])
|
||||
assert hook.wants_streaming() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Empty hooks list: behaves like no-op AgentHook
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_empty_hooks_no_ops():
|
||||
hook = CompositeHook([])
|
||||
ctx = _ctx()
|
||||
await hook.before_iteration(ctx)
|
||||
await hook.on_stream(ctx, "delta")
|
||||
await hook.on_stream_end(ctx, resuming=False)
|
||||
await hook.before_execute_tools(ctx)
|
||||
await hook.after_iteration(ctx)
|
||||
assert hook.finalize_content(ctx, "test") == "test"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_supports_legacy_hook_init_without_super():
|
||||
calls: list[str] = []
|
||||
|
||||
class LegacyHook(AgentHook):
|
||||
def __init__(self, label: str) -> None:
|
||||
self.label = label
|
||||
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
calls.append(self.label)
|
||||
|
||||
hook = CompositeHook([LegacyHook("legacy")])
|
||||
await hook.before_iteration(_ctx())
|
||||
assert calls == ["legacy"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_composite_can_wrap_another_composite():
|
||||
calls: list[str] = []
|
||||
|
||||
class Inner(AgentHook):
|
||||
async def before_iteration(self, context: AgentHookContext) -> None:
|
||||
calls.append("inner")
|
||||
|
||||
hook = CompositeHook([CompositeHook([Inner()])])
|
||||
await hook.before_iteration(_ctx())
|
||||
assert calls == ["inner"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Integration: AgentLoop with extra hooks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_loop(tmp_path, hooks=None):
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.queue import MessageBus
|
||||
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.generation.max_tokens = 4096
|
||||
|
||||
with patch("nanobot.agent.loop.ContextBuilder"), \
|
||||
patch("nanobot.agent.loop.SessionManager"), \
|
||||
patch("nanobot.agent.loop.SubagentManager") as mock_sub_mgr, \
|
||||
patch("nanobot.agent.loop.Consolidator"), \
|
||||
patch("nanobot.agent.loop.Dream"):
|
||||
mock_sub_mgr.return_value.cancel_by_session = AsyncMock(return_value=0)
|
||||
loop = AgentLoop(
|
||||
bus=bus, provider=provider, workspace=tmp_path, hooks=hooks,
|
||||
)
|
||||
return loop
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_loop_extra_hook_receives_calls(tmp_path):
|
||||
"""Extra hook passed to AgentLoop is called alongside core LoopHook."""
|
||||
from nanobot.providers.base import LLMResponse
|
||||
|
||||
events: list[str] = []
|
||||
|
||||
class TrackingHook(AgentHook):
|
||||
async def before_iteration(self, context):
|
||||
events.append(f"before_iter:{context.iteration}")
|
||||
|
||||
async def after_iteration(self, context):
|
||||
events.append(f"after_iter:{context.iteration}")
|
||||
|
||||
loop = _make_loop(tmp_path, hooks=[TrackingHook()])
|
||||
loop.provider.chat_with_retry = AsyncMock(
|
||||
return_value=LLMResponse(content="done", tool_calls=[], usage={})
|
||||
)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
|
||||
content, tools_used, messages = await loop._run_agent_loop(
|
||||
[{"role": "user", "content": "hi"}]
|
||||
)
|
||||
|
||||
assert content == "done"
|
||||
assert "before_iter:0" in events
|
||||
assert "after_iter:0" in events
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_loop_extra_hook_error_isolation(tmp_path):
|
||||
"""A faulty extra hook does not crash the agent loop."""
|
||||
from nanobot.providers.base import LLMResponse
|
||||
|
||||
class BadHook(AgentHook):
|
||||
async def before_iteration(self, context):
|
||||
raise RuntimeError("I am broken")
|
||||
|
||||
loop = _make_loop(tmp_path, hooks=[BadHook()])
|
||||
loop.provider.chat_with_retry = AsyncMock(
|
||||
return_value=LLMResponse(content="still works", tool_calls=[], usage={})
|
||||
)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
|
||||
content, _, _ = await loop._run_agent_loop(
|
||||
[{"role": "user", "content": "hi"}]
|
||||
)
|
||||
|
||||
assert content == "still works"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_loop_extra_hooks_do_not_swallow_loop_hook_errors(tmp_path):
|
||||
"""Extra hooks must not change the core LoopHook failure behavior."""
|
||||
from nanobot.providers.base import LLMResponse, ToolCallRequest
|
||||
|
||||
loop = _make_loop(tmp_path, hooks=[AgentHook()])
|
||||
loop.provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="c1", name="list_dir", arguments={"path": "."})],
|
||||
usage={},
|
||||
))
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
loop.tools.execute = AsyncMock(return_value="ok")
|
||||
|
||||
async def bad_progress(*args, **kwargs):
|
||||
raise RuntimeError("progress failed")
|
||||
|
||||
with pytest.raises(RuntimeError, match="progress failed"):
|
||||
await loop._run_agent_loop([], on_progress=bad_progress)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_loop_no_hooks_backward_compat(tmp_path):
|
||||
"""Without hooks param, behavior is identical to before."""
|
||||
from nanobot.providers.base import LLMResponse, ToolCallRequest
|
||||
|
||||
loop = _make_loop(tmp_path)
|
||||
loop.provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="c1", name="list_dir", arguments={"path": "."})],
|
||||
))
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
loop.tools.execute = AsyncMock(return_value="ok")
|
||||
loop.max_iterations = 2
|
||||
|
||||
content, tools_used, _ = await loop._run_agent_loop([])
|
||||
assert content == (
|
||||
"I reached the maximum number of tool call iterations (2) "
|
||||
"without completing the task. You can try breaking the task into smaller steps."
|
||||
)
|
||||
assert tools_used == ["list_dir", "list_dir"]
|
||||
@@ -26,24 +26,24 @@ def _make_loop(tmp_path, *, estimated_tokens: int, context_window_tokens: int) -
|
||||
context_window_tokens=context_window_tokens,
|
||||
)
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
loop.memory_consolidator._SAFETY_BUFFER = 0
|
||||
loop.consolidator._SAFETY_BUFFER = 0
|
||||
return loop
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_prompt_below_threshold_does_not_consolidate(tmp_path) -> None:
|
||||
loop = _make_loop(tmp_path, estimated_tokens=100, context_window_tokens=200)
|
||||
loop.memory_consolidator.consolidate_messages = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
loop.consolidator.archive = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
|
||||
await loop.process_direct("hello", session_key="cli:test")
|
||||
|
||||
loop.memory_consolidator.consolidate_messages.assert_not_awaited()
|
||||
loop.consolidator.archive.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_prompt_above_threshold_triggers_consolidation(tmp_path, monkeypatch) -> None:
|
||||
loop = _make_loop(tmp_path, estimated_tokens=1000, context_window_tokens=200)
|
||||
loop.memory_consolidator.consolidate_messages = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
loop.consolidator.archive = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
session = loop.sessions.get_or_create("cli:test")
|
||||
session.messages = [
|
||||
{"role": "user", "content": "u1", "timestamp": "2026-01-01T00:00:00"},
|
||||
@@ -55,13 +55,13 @@ async def test_prompt_above_threshold_triggers_consolidation(tmp_path, monkeypat
|
||||
|
||||
await loop.process_direct("hello", session_key="cli:test")
|
||||
|
||||
assert loop.memory_consolidator.consolidate_messages.await_count >= 1
|
||||
assert loop.consolidator.archive.await_count >= 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_prompt_above_threshold_archives_until_next_user_boundary(tmp_path, monkeypatch) -> None:
|
||||
loop = _make_loop(tmp_path, estimated_tokens=1000, context_window_tokens=200)
|
||||
loop.memory_consolidator.consolidate_messages = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
loop.consolidator.archive = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
|
||||
session = loop.sessions.get_or_create("cli:test")
|
||||
session.messages = [
|
||||
@@ -76,9 +76,9 @@ async def test_prompt_above_threshold_archives_until_next_user_boundary(tmp_path
|
||||
token_map = {"u1": 120, "a1": 120, "u2": 120, "a2": 120, "u3": 120}
|
||||
monkeypatch.setattr(memory_module, "estimate_message_tokens", lambda message: token_map[message["content"]])
|
||||
|
||||
await loop.memory_consolidator.maybe_consolidate_by_tokens(session)
|
||||
await loop.consolidator.maybe_consolidate_by_tokens(session)
|
||||
|
||||
archived_chunk = loop.memory_consolidator.consolidate_messages.await_args.args[0]
|
||||
archived_chunk = loop.consolidator.archive.await_args.args[0]
|
||||
assert [message["content"] for message in archived_chunk] == ["u1", "a1", "u2", "a2"]
|
||||
assert session.last_consolidated == 4
|
||||
|
||||
@@ -87,7 +87,7 @@ async def test_prompt_above_threshold_archives_until_next_user_boundary(tmp_path
|
||||
async def test_consolidation_loops_until_target_met(tmp_path, monkeypatch) -> None:
|
||||
"""Verify maybe_consolidate_by_tokens keeps looping until under threshold."""
|
||||
loop = _make_loop(tmp_path, estimated_tokens=0, context_window_tokens=200)
|
||||
loop.memory_consolidator.consolidate_messages = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
loop.consolidator.archive = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
|
||||
session = loop.sessions.get_or_create("cli:test")
|
||||
session.messages = [
|
||||
@@ -110,12 +110,12 @@ async def test_consolidation_loops_until_target_met(tmp_path, monkeypatch) -> No
|
||||
return (300, "test")
|
||||
return (80, "test")
|
||||
|
||||
loop.memory_consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
loop.consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
monkeypatch.setattr(memory_module, "estimate_message_tokens", lambda _m: 100)
|
||||
|
||||
await loop.memory_consolidator.maybe_consolidate_by_tokens(session)
|
||||
await loop.consolidator.maybe_consolidate_by_tokens(session)
|
||||
|
||||
assert loop.memory_consolidator.consolidate_messages.await_count == 2
|
||||
assert loop.consolidator.archive.await_count == 2
|
||||
assert session.last_consolidated == 6
|
||||
|
||||
|
||||
@@ -123,7 +123,7 @@ async def test_consolidation_loops_until_target_met(tmp_path, monkeypatch) -> No
|
||||
async def test_consolidation_continues_below_trigger_until_half_target(tmp_path, monkeypatch) -> None:
|
||||
"""Once triggered, consolidation should continue until it drops below half threshold."""
|
||||
loop = _make_loop(tmp_path, estimated_tokens=0, context_window_tokens=200)
|
||||
loop.memory_consolidator.consolidate_messages = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
loop.consolidator.archive = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||
|
||||
session = loop.sessions.get_or_create("cli:test")
|
||||
session.messages = [
|
||||
@@ -147,12 +147,12 @@ async def test_consolidation_continues_below_trigger_until_half_target(tmp_path,
|
||||
return (150, "test")
|
||||
return (80, "test")
|
||||
|
||||
loop.memory_consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
loop.consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
monkeypatch.setattr(memory_module, "estimate_message_tokens", lambda _m: 100)
|
||||
|
||||
await loop.memory_consolidator.maybe_consolidate_by_tokens(session)
|
||||
await loop.consolidator.maybe_consolidate_by_tokens(session)
|
||||
|
||||
assert loop.memory_consolidator.consolidate_messages.await_count == 2
|
||||
assert loop.consolidator.archive.await_count == 2
|
||||
assert session.last_consolidated == 6
|
||||
|
||||
|
||||
@@ -166,7 +166,7 @@ async def test_preflight_consolidation_before_llm_call(tmp_path, monkeypatch) ->
|
||||
async def track_consolidate(messages):
|
||||
order.append("consolidate")
|
||||
return True
|
||||
loop.memory_consolidator.consolidate_messages = track_consolidate # type: ignore[method-assign]
|
||||
loop.consolidator.archive = track_consolidate # type: ignore[method-assign]
|
||||
|
||||
async def track_llm(*args, **kwargs):
|
||||
order.append("llm")
|
||||
@@ -187,7 +187,7 @@ async def test_preflight_consolidation_before_llm_call(tmp_path, monkeypatch) ->
|
||||
def mock_estimate(_session):
|
||||
call_count[0] += 1
|
||||
return (1000 if call_count[0] <= 1 else 80, "test")
|
||||
loop.memory_consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
loop.consolidator.estimate_session_prompt_tokens = mock_estimate # type: ignore[method-assign]
|
||||
|
||||
await loop.process_direct("hello", session_key="cli:test")
|
||||
|
||||
|
||||
@@ -5,7 +5,9 @@ from nanobot.session.manager import Session
|
||||
|
||||
def _mk_loop() -> AgentLoop:
|
||||
loop = AgentLoop.__new__(AgentLoop)
|
||||
loop._TOOL_RESULT_MAX_CHARS = AgentLoop._TOOL_RESULT_MAX_CHARS
|
||||
from nanobot.config.schema import AgentDefaults
|
||||
|
||||
loop.max_tool_result_chars = AgentDefaults().max_tool_result_chars
|
||||
return loop
|
||||
|
||||
|
||||
@@ -72,3 +74,129 @@ def test_save_turn_keeps_tool_results_under_16k() -> None:
|
||||
)
|
||||
|
||||
assert session.messages[0]["content"] == content
|
||||
|
||||
|
||||
def test_restore_runtime_checkpoint_rehydrates_completed_and_pending_tools() -> None:
|
||||
loop = _mk_loop()
|
||||
session = Session(
|
||||
key="test:checkpoint",
|
||||
metadata={
|
||||
AgentLoop._RUNTIME_CHECKPOINT_KEY: {
|
||||
"assistant_message": {
|
||||
"role": "assistant",
|
||||
"content": "working",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_done",
|
||||
"type": "function",
|
||||
"function": {"name": "read_file", "arguments": "{}"},
|
||||
},
|
||||
{
|
||||
"id": "call_pending",
|
||||
"type": "function",
|
||||
"function": {"name": "exec", "arguments": "{}"},
|
||||
},
|
||||
],
|
||||
},
|
||||
"completed_tool_results": [
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_done",
|
||||
"name": "read_file",
|
||||
"content": "ok",
|
||||
}
|
||||
],
|
||||
"pending_tool_calls": [
|
||||
{
|
||||
"id": "call_pending",
|
||||
"type": "function",
|
||||
"function": {"name": "exec", "arguments": "{}"},
|
||||
}
|
||||
],
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
restored = loop._restore_runtime_checkpoint(session)
|
||||
|
||||
assert restored is True
|
||||
assert session.metadata.get(AgentLoop._RUNTIME_CHECKPOINT_KEY) is None
|
||||
assert session.messages[0]["role"] == "assistant"
|
||||
assert session.messages[1]["tool_call_id"] == "call_done"
|
||||
assert session.messages[2]["tool_call_id"] == "call_pending"
|
||||
assert "interrupted before this tool finished" in session.messages[2]["content"].lower()
|
||||
|
||||
|
||||
def test_restore_runtime_checkpoint_dedupes_overlapping_tail() -> None:
|
||||
loop = _mk_loop()
|
||||
session = Session(
|
||||
key="test:checkpoint-overlap",
|
||||
messages=[
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "working",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_done",
|
||||
"type": "function",
|
||||
"function": {"name": "read_file", "arguments": "{}"},
|
||||
},
|
||||
{
|
||||
"id": "call_pending",
|
||||
"type": "function",
|
||||
"function": {"name": "exec", "arguments": "{}"},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_done",
|
||||
"name": "read_file",
|
||||
"content": "ok",
|
||||
},
|
||||
],
|
||||
metadata={
|
||||
AgentLoop._RUNTIME_CHECKPOINT_KEY: {
|
||||
"assistant_message": {
|
||||
"role": "assistant",
|
||||
"content": "working",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_done",
|
||||
"type": "function",
|
||||
"function": {"name": "read_file", "arguments": "{}"},
|
||||
},
|
||||
{
|
||||
"id": "call_pending",
|
||||
"type": "function",
|
||||
"function": {"name": "exec", "arguments": "{}"},
|
||||
},
|
||||
],
|
||||
},
|
||||
"completed_tool_results": [
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_done",
|
||||
"name": "read_file",
|
||||
"content": "ok",
|
||||
}
|
||||
],
|
||||
"pending_tool_calls": [
|
||||
{
|
||||
"id": "call_pending",
|
||||
"type": "function",
|
||||
"function": {"name": "exec", "arguments": "{}"},
|
||||
}
|
||||
],
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
restored = loop._restore_runtime_checkpoint(session)
|
||||
|
||||
assert restored is True
|
||||
assert session.metadata.get(AgentLoop._RUNTIME_CHECKPOINT_KEY) is None
|
||||
assert len(session.messages) == 3
|
||||
assert session.messages[0]["role"] == "assistant"
|
||||
assert session.messages[1]["tool_call_id"] == "call_done"
|
||||
assert session.messages[2]["tool_call_id"] == "call_pending"
|
||||
|
||||
@@ -1,478 +0,0 @@
|
||||
"""Test MemoryStore.consolidate() handles non-string tool call arguments.
|
||||
|
||||
Regression test for https://github.com/HKUDS/nanobot/issues/1042
|
||||
When memory consolidation receives dict values instead of strings from the LLM
|
||||
tool call response, it should serialize them to JSON instead of raising TypeError.
|
||||
"""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.memory import MemoryStore
|
||||
from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest
|
||||
|
||||
|
||||
def _make_messages(message_count: int = 30):
|
||||
"""Create a list of mock messages."""
|
||||
return [
|
||||
{"role": "user", "content": f"msg{i}", "timestamp": "2026-01-01 00:00"}
|
||||
for i in range(message_count)
|
||||
]
|
||||
|
||||
|
||||
def _make_tool_response(history_entry, memory_update):
|
||||
"""Create an LLMResponse with a save_memory tool call."""
|
||||
return LLMResponse(
|
||||
content=None,
|
||||
tool_calls=[
|
||||
ToolCallRequest(
|
||||
id="call_1",
|
||||
name="save_memory",
|
||||
arguments={
|
||||
"history_entry": history_entry,
|
||||
"memory_update": memory_update,
|
||||
},
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
class ScriptedProvider(LLMProvider):
|
||||
def __init__(self, responses: list[LLMResponse]):
|
||||
super().__init__()
|
||||
self._responses = list(responses)
|
||||
self.calls = 0
|
||||
|
||||
async def chat(self, *args, **kwargs) -> LLMResponse:
|
||||
self.calls += 1
|
||||
if self._responses:
|
||||
return self._responses.pop(0)
|
||||
return LLMResponse(content="", tool_calls=[])
|
||||
|
||||
def get_default_model(self) -> str:
|
||||
return "test-model"
|
||||
|
||||
|
||||
class TestMemoryConsolidationTypeHandling:
|
||||
"""Test that consolidation handles various argument types correctly."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_string_arguments_work(self, tmp_path: Path) -> None:
|
||||
"""Normal case: LLM returns string arguments."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
provider.chat = AsyncMock(
|
||||
return_value=_make_tool_response(
|
||||
history_entry="[2026-01-01] User discussed testing.",
|
||||
memory_update="# Memory\nUser likes testing.",
|
||||
)
|
||||
)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
assert result is True
|
||||
assert store.history_file.exists()
|
||||
assert "[2026-01-01] User discussed testing." in store.history_file.read_text()
|
||||
assert "User likes testing." in store.memory_file.read_text()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dict_arguments_serialized_to_json(self, tmp_path: Path) -> None:
|
||||
"""Issue #1042: LLM returns dict instead of string — must not raise TypeError."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
provider.chat = AsyncMock(
|
||||
return_value=_make_tool_response(
|
||||
history_entry={"timestamp": "2026-01-01", "summary": "User discussed testing."},
|
||||
memory_update={"facts": ["User likes testing"], "topics": ["testing"]},
|
||||
)
|
||||
)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
assert result is True
|
||||
assert store.history_file.exists()
|
||||
history_content = store.history_file.read_text()
|
||||
parsed = json.loads(history_content.strip())
|
||||
assert parsed["summary"] == "User discussed testing."
|
||||
|
||||
memory_content = store.memory_file.read_text()
|
||||
parsed_mem = json.loads(memory_content)
|
||||
assert "User likes testing" in parsed_mem["facts"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_string_arguments_as_raw_json(self, tmp_path: Path) -> None:
|
||||
"""Some providers return arguments as a JSON string instead of parsed dict."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
|
||||
response = LLMResponse(
|
||||
content=None,
|
||||
tool_calls=[
|
||||
ToolCallRequest(
|
||||
id="call_1",
|
||||
name="save_memory",
|
||||
arguments=json.dumps({
|
||||
"history_entry": "[2026-01-01] User discussed testing.",
|
||||
"memory_update": "# Memory\nUser likes testing.",
|
||||
}),
|
||||
)
|
||||
],
|
||||
)
|
||||
provider.chat = AsyncMock(return_value=response)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
assert result is True
|
||||
assert "User discussed testing." in store.history_file.read_text()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_tool_call_returns_false(self, tmp_path: Path) -> None:
|
||||
"""When LLM doesn't use the save_memory tool, return False."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
provider.chat = AsyncMock(
|
||||
return_value=LLMResponse(content="I summarized the conversation.", tool_calls=[])
|
||||
)
|
||||
provider.chat_with_retry = provider.chat
|
||||
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_skips_when_message_chunk_is_empty(self, tmp_path: Path) -> None:
|
||||
"""Consolidation should be a no-op when the selected chunk is empty."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages: list[dict] = []
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
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()
|
||||
|
||||
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)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
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)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
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)
|
||||
provider.chat_with_retry = provider.chat
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
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)
|
||||
provider = ScriptedProvider([
|
||||
LLMResponse(content="503 server error", finish_reason="error"),
|
||||
_make_tool_response(
|
||||
history_entry="[2026-01-01] User discussed testing.",
|
||||
memory_update="# Memory\nUser likes testing.",
|
||||
),
|
||||
])
|
||||
messages = _make_messages(message_count=60)
|
||||
delays: list[int] = []
|
||||
|
||||
async def _fake_sleep(delay: int) -> None:
|
||||
delays.append(delay)
|
||||
|
||||
monkeypatch.setattr("nanobot.providers.base.asyncio.sleep", _fake_sleep)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
assert result is True
|
||||
assert provider.calls == 2
|
||||
assert delays == [1]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_consolidation_delegates_to_provider_defaults(self, tmp_path: Path) -> None:
|
||||
"""Consolidation no longer passes generation params — the provider owns them."""
|
||||
store = MemoryStore(tmp_path)
|
||||
provider = AsyncMock()
|
||||
provider.chat_with_retry = AsyncMock(
|
||||
return_value=_make_tool_response(
|
||||
history_entry="[2026-01-01] User discussed testing.",
|
||||
memory_update="# Memory\nUser likes testing.",
|
||||
)
|
||||
)
|
||||
messages = _make_messages(message_count=60)
|
||||
|
||||
result = await store.consolidate(messages, provider, "test-model")
|
||||
|
||||
assert result is True
|
||||
provider.chat_with_retry.assert_awaited_once()
|
||||
_, kwargs = provider.chat_with_retry.await_args
|
||||
assert kwargs["model"] == "test-model"
|
||||
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: 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
|
||||
@@ -0,0 +1,267 @@
|
||||
"""Tests for the restructured MemoryStore — pure file I/O layer."""
|
||||
|
||||
from datetime import datetime
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.memory import MemoryStore
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store(tmp_path):
|
||||
return MemoryStore(tmp_path)
|
||||
|
||||
|
||||
class TestMemoryStoreBasicIO:
|
||||
def test_read_memory_returns_empty_when_missing(self, store):
|
||||
assert store.read_memory() == ""
|
||||
|
||||
def test_write_and_read_memory(self, store):
|
||||
store.write_memory("hello")
|
||||
assert store.read_memory() == "hello"
|
||||
|
||||
def test_read_soul_returns_empty_when_missing(self, store):
|
||||
assert store.read_soul() == ""
|
||||
|
||||
def test_write_and_read_soul(self, store):
|
||||
store.write_soul("soul content")
|
||||
assert store.read_soul() == "soul content"
|
||||
|
||||
def test_read_user_returns_empty_when_missing(self, store):
|
||||
assert store.read_user() == ""
|
||||
|
||||
def test_write_and_read_user(self, store):
|
||||
store.write_user("user content")
|
||||
assert store.read_user() == "user content"
|
||||
|
||||
def test_get_memory_context_returns_empty_when_missing(self, store):
|
||||
assert store.get_memory_context() == ""
|
||||
|
||||
def test_get_memory_context_returns_formatted_content(self, store):
|
||||
store.write_memory("important fact")
|
||||
ctx = store.get_memory_context()
|
||||
assert "Long-term Memory" in ctx
|
||||
assert "important fact" in ctx
|
||||
|
||||
|
||||
class TestHistoryWithCursor:
|
||||
def test_append_history_returns_cursor(self, store):
|
||||
cursor = store.append_history("event 1")
|
||||
assert cursor == 1
|
||||
cursor2 = store.append_history("event 2")
|
||||
assert cursor2 == 2
|
||||
|
||||
def test_append_history_includes_cursor_in_file(self, store):
|
||||
store.append_history("event 1")
|
||||
content = store.read_file(store.history_file)
|
||||
data = json.loads(content)
|
||||
assert data["cursor"] == 1
|
||||
|
||||
def test_cursor_persists_across_appends(self, store):
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
cursor = store.append_history("event 3")
|
||||
assert cursor == 3
|
||||
|
||||
def test_read_unprocessed_history(self, store):
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
store.append_history("event 3")
|
||||
entries = store.read_unprocessed_history(since_cursor=1)
|
||||
assert len(entries) == 2
|
||||
assert entries[0]["cursor"] == 2
|
||||
|
||||
def test_read_unprocessed_history_returns_all_when_cursor_zero(self, store):
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 2
|
||||
|
||||
def test_compact_history_drops_oldest(self, tmp_path):
|
||||
store = MemoryStore(tmp_path, max_history_entries=2)
|
||||
store.append_history("event 1")
|
||||
store.append_history("event 2")
|
||||
store.append_history("event 3")
|
||||
store.append_history("event 4")
|
||||
store.append_history("event 5")
|
||||
store.compact_history()
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 2
|
||||
assert entries[0]["cursor"] in {4, 5}
|
||||
|
||||
|
||||
class TestDreamCursor:
|
||||
def test_initial_cursor_is_zero(self, store):
|
||||
assert store.get_last_dream_cursor() == 0
|
||||
|
||||
def test_set_and_get_cursor(self, store):
|
||||
store.set_last_dream_cursor(5)
|
||||
assert store.get_last_dream_cursor() == 5
|
||||
|
||||
def test_cursor_persists(self, store):
|
||||
store.set_last_dream_cursor(3)
|
||||
store2 = MemoryStore(store.workspace)
|
||||
assert store2.get_last_dream_cursor() == 3
|
||||
|
||||
|
||||
class TestLegacyHistoryMigration:
|
||||
def test_read_unprocessed_history_handles_entries_without_cursor(self, store):
|
||||
"""JSONL entries with cursor=1 are correctly parsed and returned."""
|
||||
store.history_file.write_text(
|
||||
'{"cursor": 1, "timestamp": "2026-03-30 14:30", "content": "Old event"}\n',
|
||||
encoding="utf-8")
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["cursor"] == 1
|
||||
|
||||
def test_migrates_legacy_history_md_preserving_partial_entries(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_content = (
|
||||
"[2026-04-01 10:00] User prefers dark mode.\n\n"
|
||||
"[2026-04-01 10:05] [RAW] 2 messages\n"
|
||||
"[2026-04-01 10:04] USER: hello\n"
|
||||
"[2026-04-01 10:04] ASSISTANT: hi\n\n"
|
||||
"Legacy chunk without timestamp.\n"
|
||||
"Keep whatever content we can recover.\n"
|
||||
)
|
||||
legacy_file.write_text(legacy_content, encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
fallback_timestamp = datetime.fromtimestamp(
|
||||
(memory_dir / "HISTORY.md.bak").stat().st_mtime,
|
||||
).strftime("%Y-%m-%d %H:%M")
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert [entry["cursor"] for entry in entries] == [1, 2, 3]
|
||||
assert entries[0]["timestamp"] == "2026-04-01 10:00"
|
||||
assert entries[0]["content"] == "User prefers dark mode."
|
||||
assert entries[1]["timestamp"] == "2026-04-01 10:05"
|
||||
assert entries[1]["content"].startswith("[RAW] 2 messages")
|
||||
assert "USER: hello" in entries[1]["content"]
|
||||
assert entries[2]["timestamp"] == fallback_timestamp
|
||||
assert entries[2]["content"].startswith("Legacy chunk without timestamp.")
|
||||
assert store.read_file(store._cursor_file).strip() == "3"
|
||||
assert store.read_file(store._dream_cursor_file).strip() == "3"
|
||||
assert not legacy_file.exists()
|
||||
assert (memory_dir / "HISTORY.md.bak").read_text(encoding="utf-8") == legacy_content
|
||||
|
||||
def test_migrates_consecutive_entries_without_blank_lines(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_content = (
|
||||
"[2026-04-01 10:00] First event.\n"
|
||||
"[2026-04-01 10:01] Second event.\n"
|
||||
"[2026-04-01 10:02] Third event.\n"
|
||||
)
|
||||
legacy_file.write_text(legacy_content, encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 3
|
||||
assert [entry["content"] for entry in entries] == [
|
||||
"First event.",
|
||||
"Second event.",
|
||||
"Third event.",
|
||||
]
|
||||
|
||||
def test_raw_archive_stays_single_entry_while_following_events_split(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_content = (
|
||||
"[2026-04-01 10:05] [RAW] 2 messages\n"
|
||||
"[2026-04-01 10:04] USER: hello\n"
|
||||
"[2026-04-01 10:04] ASSISTANT: hi\n"
|
||||
"[2026-04-01 10:06] Normal event after raw block.\n"
|
||||
)
|
||||
legacy_file.write_text(legacy_content, encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 2
|
||||
assert entries[0]["content"].startswith("[RAW] 2 messages")
|
||||
assert "USER: hello" in entries[0]["content"]
|
||||
assert entries[1]["content"] == "Normal event after raw block."
|
||||
|
||||
def test_nonstandard_date_headers_still_start_new_entries(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_content = (
|
||||
"[2026-03-25–2026-04-02] Multi-day summary.\n"
|
||||
"[2026-03-26/27] Cross-day summary.\n"
|
||||
)
|
||||
legacy_file.write_text(legacy_content, encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
fallback_timestamp = datetime.fromtimestamp(
|
||||
(memory_dir / "HISTORY.md.bak").stat().st_mtime,
|
||||
).strftime("%Y-%m-%d %H:%M")
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 2
|
||||
assert entries[0]["timestamp"] == fallback_timestamp
|
||||
assert entries[0]["content"] == "[2026-03-25–2026-04-02] Multi-day summary."
|
||||
assert entries[1]["timestamp"] == fallback_timestamp
|
||||
assert entries[1]["content"] == "[2026-03-26/27] Cross-day summary."
|
||||
|
||||
def test_existing_history_jsonl_skips_legacy_migration(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
history_file = memory_dir / "history.jsonl"
|
||||
history_file.write_text(
|
||||
'{"cursor": 7, "timestamp": "2026-04-01 12:00", "content": "existing"}\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_file.write_text("[2026-04-01 10:00] legacy\n\n", encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["cursor"] == 7
|
||||
assert entries[0]["content"] == "existing"
|
||||
assert legacy_file.exists()
|
||||
assert not (memory_dir / "HISTORY.md.bak").exists()
|
||||
|
||||
def test_empty_history_jsonl_still_allows_legacy_migration(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
history_file = memory_dir / "history.jsonl"
|
||||
history_file.write_text("", encoding="utf-8")
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_file.write_text("[2026-04-01 10:00] legacy\n\n", encoding="utf-8")
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["cursor"] == 1
|
||||
assert entries[0]["timestamp"] == "2026-04-01 10:00"
|
||||
assert entries[0]["content"] == "legacy"
|
||||
assert not legacy_file.exists()
|
||||
assert (memory_dir / "HISTORY.md.bak").exists()
|
||||
|
||||
def test_migrates_legacy_history_with_invalid_utf8_bytes(self, tmp_path):
|
||||
memory_dir = tmp_path / "memory"
|
||||
memory_dir.mkdir()
|
||||
legacy_file = memory_dir / "HISTORY.md"
|
||||
legacy_file.write_bytes(
|
||||
b"[2026-04-01 10:00] Broken \xff data still needs migration.\n\n"
|
||||
)
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
|
||||
entries = store.read_unprocessed_history(since_cursor=0)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["timestamp"] == "2026-04-01 10:00"
|
||||
assert "Broken" in entries[0]["content"]
|
||||
assert "migration." in entries[0]["content"]
|
||||
+924
-5
File diff suppressed because it is too large
Load Diff
@@ -173,6 +173,27 @@ def test_empty_session_history():
|
||||
assert history == []
|
||||
|
||||
|
||||
def test_get_history_preserves_reasoning_content():
|
||||
session = Session(key="test:reasoning")
|
||||
session.messages.append({"role": "user", "content": "hi"})
|
||||
session.messages.append({
|
||||
"role": "assistant",
|
||||
"content": "done",
|
||||
"reasoning_content": "hidden chain of thought",
|
||||
})
|
||||
|
||||
history = session.get_history(max_messages=500)
|
||||
|
||||
assert history == [
|
||||
{"role": "user", "content": "hi"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "done",
|
||||
"reasoning_content": "hidden chain of thought",
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
# --- Window cuts mid-group: assistant present but some tool results orphaned ---
|
||||
|
||||
def test_window_cuts_mid_tool_group():
|
||||
|
||||
@@ -0,0 +1,252 @@
|
||||
"""Tests for nanobot.agent.skills.SkillsLoader."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.skills import SkillsLoader
|
||||
|
||||
|
||||
def _write_skill(
|
||||
base: Path,
|
||||
name: str,
|
||||
*,
|
||||
metadata_json: dict | None = None,
|
||||
body: str = "# Skill\n",
|
||||
) -> Path:
|
||||
"""Create ``base / name / SKILL.md`` with optional nanobot metadata JSON."""
|
||||
skill_dir = base / name
|
||||
skill_dir.mkdir(parents=True)
|
||||
lines = ["---"]
|
||||
if metadata_json is not None:
|
||||
payload = json.dumps({"nanobot": metadata_json}, separators=(",", ":"))
|
||||
lines.append(f'metadata: {payload}')
|
||||
lines.extend(["---", "", body])
|
||||
path = skill_dir / "SKILL.md"
|
||||
path.write_text("\n".join(lines), encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
def test_list_skills_empty_when_skills_dir_missing(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
assert loader.list_skills(filter_unavailable=False) == []
|
||||
|
||||
|
||||
def test_list_skills_empty_when_skills_dir_exists_but_empty(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
(workspace / "skills").mkdir(parents=True)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
assert loader.list_skills(filter_unavailable=False) == []
|
||||
|
||||
|
||||
def test_list_skills_workspace_entry_shape_and_source(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
skill_path = _write_skill(skills_root, "alpha", body="# Alpha")
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = loader.list_skills(filter_unavailable=False)
|
||||
assert entries == [
|
||||
{"name": "alpha", "path": str(skill_path), "source": "workspace"},
|
||||
]
|
||||
|
||||
|
||||
def test_list_skills_skips_non_directories_and_missing_skill_md(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
(skills_root / "not_a_dir.txt").write_text("x", encoding="utf-8")
|
||||
(skills_root / "no_skill_md").mkdir()
|
||||
ok_path = _write_skill(skills_root, "ok", body="# Ok")
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = loader.list_skills(filter_unavailable=False)
|
||||
names = {entry["name"] for entry in entries}
|
||||
assert names == {"ok"}
|
||||
assert entries[0]["path"] == str(ok_path)
|
||||
|
||||
|
||||
def test_list_skills_workspace_shadows_builtin_same_name(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
ws_skills = workspace / "skills"
|
||||
ws_skills.mkdir(parents=True)
|
||||
ws_path = _write_skill(ws_skills, "dup", body="# Workspace wins")
|
||||
|
||||
builtin = tmp_path / "builtin"
|
||||
_write_skill(builtin, "dup", body="# Builtin")
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = loader.list_skills(filter_unavailable=False)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["source"] == "workspace"
|
||||
assert entries[0]["path"] == str(ws_path)
|
||||
|
||||
|
||||
def test_list_skills_merges_workspace_and_builtin(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
ws_skills = workspace / "skills"
|
||||
ws_skills.mkdir(parents=True)
|
||||
ws_path = _write_skill(ws_skills, "ws_only", body="# W")
|
||||
builtin = tmp_path / "builtin"
|
||||
bi_path = _write_skill(builtin, "bi_only", body="# B")
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = sorted(loader.list_skills(filter_unavailable=False), key=lambda item: item["name"])
|
||||
assert entries == [
|
||||
{"name": "bi_only", "path": str(bi_path), "source": "builtin"},
|
||||
{"name": "ws_only", "path": str(ws_path), "source": "workspace"},
|
||||
]
|
||||
|
||||
|
||||
def test_list_skills_builtin_omitted_when_dir_missing(tmp_path: Path) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
ws_skills = workspace / "skills"
|
||||
ws_skills.mkdir(parents=True)
|
||||
ws_path = _write_skill(ws_skills, "solo", body="# S")
|
||||
missing_builtin = tmp_path / "no_such_builtin"
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=missing_builtin)
|
||||
entries = loader.list_skills(filter_unavailable=False)
|
||||
assert entries == [{"name": "solo", "path": str(ws_path), "source": "workspace"}]
|
||||
|
||||
|
||||
def test_list_skills_filter_unavailable_excludes_unmet_bin_requirement(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
_write_skill(
|
||||
skills_root,
|
||||
"needs_bin",
|
||||
metadata_json={"requires": {"bins": ["nanobot_test_fake_binary"]}},
|
||||
)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
def fake_which(cmd: str) -> str | None:
|
||||
if cmd == "nanobot_test_fake_binary":
|
||||
return None
|
||||
return "/usr/bin/true"
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.skills.shutil.which", fake_which)
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
assert loader.list_skills(filter_unavailable=True) == []
|
||||
|
||||
|
||||
def test_list_skills_filter_unavailable_includes_when_bin_requirement_met(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
skill_path = _write_skill(
|
||||
skills_root,
|
||||
"has_bin",
|
||||
metadata_json={"requires": {"bins": ["nanobot_test_fake_binary"]}},
|
||||
)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
def fake_which(cmd: str) -> str | None:
|
||||
if cmd == "nanobot_test_fake_binary":
|
||||
return "/fake/nanobot_test_fake_binary"
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.skills.shutil.which", fake_which)
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = loader.list_skills(filter_unavailable=True)
|
||||
assert entries == [
|
||||
{"name": "has_bin", "path": str(skill_path), "source": "workspace"},
|
||||
]
|
||||
|
||||
|
||||
def test_list_skills_filter_unavailable_false_keeps_unmet_requirements(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
skill_path = _write_skill(
|
||||
skills_root,
|
||||
"blocked",
|
||||
metadata_json={"requires": {"bins": ["nanobot_test_fake_binary"]}},
|
||||
)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.skills.shutil.which", lambda _cmd: None)
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
entries = loader.list_skills(filter_unavailable=False)
|
||||
assert entries == [
|
||||
{"name": "blocked", "path": str(skill_path), "source": "workspace"},
|
||||
]
|
||||
|
||||
|
||||
def test_list_skills_filter_unavailable_excludes_unmet_env_requirement(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
_write_skill(
|
||||
skills_root,
|
||||
"needs_env",
|
||||
metadata_json={"requires": {"env": ["NANOBOT_SKILLS_TEST_ENV_VAR"]}},
|
||||
)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
monkeypatch.delenv("NANOBOT_SKILLS_TEST_ENV_VAR", raising=False)
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
assert loader.list_skills(filter_unavailable=True) == []
|
||||
|
||||
|
||||
def test_list_skills_openclaw_metadata_parsed_for_requirements(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
workspace = tmp_path / "ws"
|
||||
skills_root = workspace / "skills"
|
||||
skills_root.mkdir(parents=True)
|
||||
skill_dir = skills_root / "openclaw_skill"
|
||||
skill_dir.mkdir(parents=True)
|
||||
skill_path = skill_dir / "SKILL.md"
|
||||
oc_payload = json.dumps({"openclaw": {"requires": {"bins": ["nanobot_oc_bin"]}}}, separators=(",", ":"))
|
||||
skill_path.write_text(
|
||||
"\n".join(["---", f"metadata: {oc_payload}", "---", "", "# OC"]),
|
||||
encoding="utf-8",
|
||||
)
|
||||
builtin = tmp_path / "builtin"
|
||||
builtin.mkdir()
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.skills.shutil.which", lambda _cmd: None)
|
||||
|
||||
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||
assert loader.list_skills(filter_unavailable=True) == []
|
||||
|
||||
monkeypatch.setattr(
|
||||
"nanobot.agent.skills.shutil.which",
|
||||
lambda cmd: "/x" if cmd == "nanobot_oc_bin" else None,
|
||||
)
|
||||
entries = loader.list_skills(filter_unavailable=True)
|
||||
assert entries == [
|
||||
{"name": "openclaw_skill", "path": str(skill_path), "source": "workspace"},
|
||||
]
|
||||
+116
-15
@@ -3,10 +3,15 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.config.schema import AgentDefaults
|
||||
|
||||
_MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars
|
||||
|
||||
|
||||
def _make_loop(*, exec_config=None):
|
||||
"""Create a minimal AgentLoop with mocked dependencies."""
|
||||
@@ -116,6 +121,43 @@ class TestDispatch:
|
||||
out = await asyncio.wait_for(bus.consume_outbound(), timeout=1.0)
|
||||
assert out.content == "hi"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_streaming_preserves_message_metadata(self):
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
loop, bus = _make_loop()
|
||||
msg = InboundMessage(
|
||||
channel="matrix",
|
||||
sender_id="u1",
|
||||
chat_id="!room:matrix.org",
|
||||
content="hello",
|
||||
metadata={
|
||||
"_wants_stream": True,
|
||||
"thread_root_event_id": "$root1",
|
||||
"thread_reply_to_event_id": "$reply1",
|
||||
},
|
||||
)
|
||||
|
||||
async def fake_process(_msg, *, on_stream=None, on_stream_end=None, **kwargs):
|
||||
assert on_stream is not None
|
||||
assert on_stream_end is not None
|
||||
await on_stream("hi")
|
||||
await on_stream_end(resuming=False)
|
||||
return None
|
||||
|
||||
loop._process_message = fake_process
|
||||
|
||||
await loop._dispatch(msg)
|
||||
first = await asyncio.wait_for(bus.consume_outbound(), timeout=1.0)
|
||||
second = await asyncio.wait_for(bus.consume_outbound(), timeout=1.0)
|
||||
|
||||
assert first.metadata["thread_root_event_id"] == "$root1"
|
||||
assert first.metadata["thread_reply_to_event_id"] == "$reply1"
|
||||
assert first.metadata["_stream_delta"] is True
|
||||
assert second.metadata["thread_root_event_id"] == "$root1"
|
||||
assert second.metadata["thread_reply_to_event_id"] == "$reply1"
|
||||
assert second.metadata["_stream_end"] is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_processing_lock_serializes(self):
|
||||
from nanobot.bus.events import InboundMessage, OutboundMessage
|
||||
@@ -148,7 +190,12 @@ class TestSubagentCancellation:
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
mgr = SubagentManager(provider=provider, workspace=MagicMock(), bus=bus)
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=MagicMock(),
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
)
|
||||
|
||||
cancelled = asyncio.Event()
|
||||
|
||||
@@ -176,7 +223,12 @@ class TestSubagentCancellation:
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
mgr = SubagentManager(provider=provider, workspace=MagicMock(), bus=bus)
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=MagicMock(),
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
)
|
||||
assert await mgr.cancel_by_session("nonexistent") == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -198,19 +250,24 @@ class TestSubagentCancellation:
|
||||
if call_count["n"] == 1:
|
||||
return LLMResponse(
|
||||
content="thinking",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={"path": "."})],
|
||||
reasoning_content="hidden reasoning",
|
||||
thinking_blocks=[{"type": "thinking", "thinking": "step"}],
|
||||
)
|
||||
captured_second_call[:] = messages
|
||||
return LLMResponse(content="done", tool_calls=[])
|
||||
provider.chat_with_retry = scripted_chat_with_retry
|
||||
mgr = SubagentManager(provider=provider, workspace=tmp_path, bus=bus)
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
)
|
||||
|
||||
async def fake_execute(self, name, arguments):
|
||||
async def fake_execute(self, **kwargs):
|
||||
return "tool result"
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.tools.registry.ToolRegistry.execute", fake_execute)
|
||||
monkeypatch.setattr("nanobot.agent.tools.filesystem.ListDirTool.execute", fake_execute)
|
||||
|
||||
await mgr._run_subagent("sub-1", "do task", "label", {"channel": "test", "chat_id": "c1"})
|
||||
|
||||
@@ -222,6 +279,40 @@ class TestSubagentCancellation:
|
||||
assert assistant_messages[0]["reasoning_content"] == "hidden reasoning"
|
||||
assert assistant_messages[0]["thinking_blocks"] == [{"type": "thinking", "thinking": "step"}]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subagent_exec_tool_not_registered_when_disabled(self, tmp_path):
|
||||
from nanobot.agent.subagent import SubagentManager
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.schema import ExecToolConfig
|
||||
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
exec_config=ExecToolConfig(enable=False),
|
||||
)
|
||||
mgr._announce_result = AsyncMock()
|
||||
|
||||
async def fake_run(spec):
|
||||
assert spec.tools.get("exec") is None
|
||||
return SimpleNamespace(
|
||||
stop_reason="done",
|
||||
final_content="done",
|
||||
error=None,
|
||||
tool_events=[],
|
||||
)
|
||||
|
||||
mgr.runner.run = AsyncMock(side_effect=fake_run)
|
||||
|
||||
await mgr._run_subagent("sub-1", "do task", "label", {"channel": "test", "chat_id": "c1"})
|
||||
|
||||
mgr.runner.run.assert_awaited_once()
|
||||
mgr._announce_result.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subagent_announces_error_when_tool_execution_fails(self, monkeypatch, tmp_path):
|
||||
from nanobot.agent.subagent import SubagentManager
|
||||
@@ -233,20 +324,25 @@ class TestSubagentCancellation:
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="thinking",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={"path": "."})],
|
||||
))
|
||||
mgr = SubagentManager(provider=provider, workspace=tmp_path, bus=bus)
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
)
|
||||
mgr._announce_result = AsyncMock()
|
||||
|
||||
calls = {"n": 0}
|
||||
|
||||
async def fake_execute(self, name, arguments):
|
||||
async def fake_execute(self, **kwargs):
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 1:
|
||||
return "first result"
|
||||
raise RuntimeError("boom")
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.tools.registry.ToolRegistry.execute", fake_execute)
|
||||
monkeypatch.setattr("nanobot.agent.tools.filesystem.ListDirTool.execute", fake_execute)
|
||||
|
||||
await mgr._run_subagent("sub-1", "do task", "label", {"channel": "test", "chat_id": "c1"})
|
||||
|
||||
@@ -269,15 +365,20 @@ class TestSubagentCancellation:
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="thinking",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={"path": "."})],
|
||||
))
|
||||
mgr = SubagentManager(provider=provider, workspace=tmp_path, bus=bus)
|
||||
mgr = SubagentManager(
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
bus=bus,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
)
|
||||
mgr._announce_result = AsyncMock()
|
||||
|
||||
started = asyncio.Event()
|
||||
cancelled = asyncio.Event()
|
||||
|
||||
async def fake_execute(self, name, arguments):
|
||||
async def fake_execute(self, **kwargs):
|
||||
started.set()
|
||||
try:
|
||||
await asyncio.sleep(60)
|
||||
@@ -285,7 +386,7 @@ class TestSubagentCancellation:
|
||||
cancelled.set()
|
||||
raise
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.tools.registry.ToolRegistry.execute", fake_execute)
|
||||
monkeypatch.setattr("nanobot.agent.tools.filesystem.ListDirTool.execute", fake_execute)
|
||||
|
||||
task = asyncio.create_task(
|
||||
mgr._run_subagent("sub-1", "do task", "label", {"channel": "test", "chat_id": "c1"})
|
||||
@@ -293,7 +394,7 @@ class TestSubagentCancellation:
|
||||
mgr._running_tasks["sub-1"] = task
|
||||
mgr._session_tasks["test:c1"] = {"sub-1"}
|
||||
|
||||
await started.wait()
|
||||
await asyncio.wait_for(started.wait(), timeout=1.0)
|
||||
|
||||
count = await mgr.cancel_by_session("test:c1")
|
||||
|
||||
|
||||
@@ -0,0 +1,256 @@
|
||||
"""Tests for tool hint formatting (nanobot.utils.tool_hints)."""
|
||||
|
||||
from nanobot.utils.tool_hints import format_tool_hints
|
||||
from nanobot.providers.base import ToolCallRequest
|
||||
|
||||
|
||||
def _tc(name: str, args) -> ToolCallRequest:
|
||||
return ToolCallRequest(id="c1", name=name, arguments=args)
|
||||
|
||||
|
||||
def _hint(calls):
|
||||
"""Shortcut for format_tool_hints."""
|
||||
return format_tool_hints(calls)
|
||||
|
||||
|
||||
class TestToolHintKnownTools:
|
||||
"""Test registered tool types produce correct formatted output."""
|
||||
|
||||
def test_read_file_short_path(self):
|
||||
result = _hint([_tc("read_file", {"path": "foo.txt"})])
|
||||
assert result == 'read foo.txt'
|
||||
|
||||
def test_read_file_long_path(self):
|
||||
result = _hint([_tc("read_file", {"path": "/home/user/.local/share/uv/tools/nanobot/agent/loop.py"})])
|
||||
assert "loop.py" in result
|
||||
assert "read " in result
|
||||
|
||||
def test_write_file_shows_path_not_content(self):
|
||||
result = _hint([_tc("write_file", {"path": "docs/api.md", "content": "# API Reference\n\nLong content..."})])
|
||||
assert result == "write docs/api.md"
|
||||
|
||||
def test_edit_shows_path(self):
|
||||
result = _hint([_tc("edit", {"file_path": "src/main.py", "old_string": "x", "new_string": "y"})])
|
||||
assert "main.py" in result
|
||||
assert "edit " in result
|
||||
|
||||
def test_glob_shows_pattern(self):
|
||||
result = _hint([_tc("glob", {"pattern": "**/*.py", "path": "src"})])
|
||||
assert result == 'glob "**/*.py"'
|
||||
|
||||
def test_grep_shows_pattern(self):
|
||||
result = _hint([_tc("grep", {"pattern": "TODO|FIXME", "path": "src"})])
|
||||
assert result == 'grep "TODO|FIXME"'
|
||||
|
||||
def test_exec_shows_command(self):
|
||||
result = _hint([_tc("exec", {"command": "npm install typescript"})])
|
||||
assert result == "$ npm install typescript"
|
||||
|
||||
def test_exec_truncates_long_command(self):
|
||||
cmd = "cd /very/long/path && cat file && echo done && sleep 1 && ls -la"
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert result.startswith("$ ")
|
||||
assert len(result) <= 50 # reasonable limit
|
||||
|
||||
def test_exec_abbreviates_paths_in_command(self):
|
||||
"""Windows paths in exec commands should be folded, not blindly truncated."""
|
||||
cmd = "cd D:\\Documents\\GitHub\\nanobot\\.worktree\\tomain\\nanobot && git diff origin/main...pr-2706 --name-only 2>&1"
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result # path should be folded with …/
|
||||
assert "worktree" not in result # middle segments should be collapsed
|
||||
|
||||
def test_exec_abbreviates_linux_paths(self):
|
||||
"""Unix absolute paths in exec commands should be folded."""
|
||||
cmd = "cd /home/user/projects/nanobot/.worktree/tomain && make build"
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result
|
||||
assert "projects" not in result
|
||||
|
||||
def test_exec_abbreviates_home_paths(self):
|
||||
"""~/ paths in exec commands should be folded."""
|
||||
cmd = "cd ~/projects/nanobot/workspace && pytest tests/"
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result
|
||||
|
||||
def test_exec_abbreviates_quoted_linux_paths_with_spaces(self):
|
||||
"""Quoted Unix paths with spaces should still be folded."""
|
||||
cmd = 'cd "/home/user/My Documents/project" && pytest tests/'
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result
|
||||
assert '"/home/user/My Documents/project"' not in result
|
||||
assert '"' in result
|
||||
|
||||
def test_exec_abbreviates_quoted_windows_paths_with_spaces(self):
|
||||
"""Quoted Windows paths with spaces should still be folded."""
|
||||
cmd = 'cd "C:/Program Files/Git/project" && git status'
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result
|
||||
assert '"C:/Program Files/Git/project"' not in result
|
||||
assert '"' in result
|
||||
|
||||
def test_exec_short_command_unchanged(self):
|
||||
result = _hint([_tc("exec", {"command": "npm install typescript"})])
|
||||
assert result == "$ npm install typescript"
|
||||
|
||||
def test_exec_chained_commands_truncated_not_mid_path(self):
|
||||
"""Long chained commands should truncate preserving abbreviated paths."""
|
||||
cmd = "cd D:\\Documents\\GitHub\\project && npm run build && npm test"
|
||||
result = _hint([_tc("exec", {"command": cmd})])
|
||||
assert "\u2026/" in result # path folded
|
||||
assert "npm" in result # chained command still visible
|
||||
|
||||
def test_web_search(self):
|
||||
result = _hint([_tc("web_search", {"query": "Claude 4 vs GPT-4"})])
|
||||
assert result == 'search "Claude 4 vs GPT-4"'
|
||||
|
||||
def test_web_fetch(self):
|
||||
result = _hint([_tc("web_fetch", {"url": "https://example.com/page"})])
|
||||
assert result == "fetch https://example.com/page"
|
||||
|
||||
|
||||
class TestToolHintMCP:
|
||||
"""Test MCP tools are abbreviated to server::tool format."""
|
||||
|
||||
def test_mcp_standard_format(self):
|
||||
result = _hint([_tc("mcp_4_5v_mcp__analyze_image", {"imageSource": "https://img.jpg", "prompt": "describe"})])
|
||||
assert "4_5v" in result
|
||||
assert "analyze_image" in result
|
||||
|
||||
def test_mcp_simple_name(self):
|
||||
result = _hint([_tc("mcp_github__create_issue", {"title": "Bug fix"})])
|
||||
assert "github" in result
|
||||
assert "create_issue" in result
|
||||
|
||||
|
||||
class TestToolHintFallback:
|
||||
"""Test unknown tools fall back to original behavior."""
|
||||
|
||||
def test_unknown_tool_with_string_arg(self):
|
||||
result = _hint([_tc("custom_tool", {"data": "hello world"})])
|
||||
assert result == 'custom_tool("hello world")'
|
||||
|
||||
def test_unknown_tool_with_long_arg_truncates(self):
|
||||
long_val = "a" * 60
|
||||
result = _hint([_tc("custom_tool", {"data": long_val})])
|
||||
assert len(result) < 80
|
||||
assert "\u2026" in result
|
||||
|
||||
def test_unknown_tool_no_string_arg(self):
|
||||
result = _hint([_tc("custom_tool", {"count": 42})])
|
||||
assert result == "custom_tool"
|
||||
|
||||
def test_empty_tool_calls(self):
|
||||
result = _hint([])
|
||||
assert result == ""
|
||||
|
||||
|
||||
class TestToolHintFolding:
|
||||
"""Test consecutive same-tool calls are folded."""
|
||||
|
||||
def test_single_call_no_fold(self):
|
||||
calls = [_tc("grep", {"pattern": "*.py"})]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
|
||||
def test_two_consecutive_different_args_not_folded(self):
|
||||
calls = [
|
||||
_tc("grep", {"pattern": "*.py"}),
|
||||
_tc("grep", {"pattern": "*.ts"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
|
||||
def test_two_consecutive_same_args_folded(self):
|
||||
calls = [
|
||||
_tc("grep", {"pattern": "TODO"}),
|
||||
_tc("grep", {"pattern": "TODO"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7 2" in result
|
||||
|
||||
def test_three_consecutive_different_args_not_folded(self):
|
||||
calls = [
|
||||
_tc("read_file", {"path": "a.py"}),
|
||||
_tc("read_file", {"path": "b.py"}),
|
||||
_tc("read_file", {"path": "c.py"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
|
||||
def test_different_tools_not_folded(self):
|
||||
calls = [
|
||||
_tc("grep", {"pattern": "TODO"}),
|
||||
_tc("read_file", {"path": "a.py"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
|
||||
def test_interleaved_same_tools_not_folded(self):
|
||||
calls = [
|
||||
_tc("grep", {"pattern": "a"}),
|
||||
_tc("read_file", {"path": "f.py"}),
|
||||
_tc("grep", {"pattern": "b"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
|
||||
|
||||
class TestToolHintMultipleCalls:
|
||||
"""Test multiple different tool calls are comma-separated."""
|
||||
|
||||
def test_two_different_tools(self):
|
||||
calls = [
|
||||
_tc("grep", {"pattern": "TODO"}),
|
||||
_tc("read_file", {"path": "main.py"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert 'grep "TODO"' in result
|
||||
assert "read main.py" in result
|
||||
assert ", " in result
|
||||
|
||||
|
||||
class TestToolHintEdgeCases:
|
||||
"""Test edge cases and defensive handling (G1, G2)."""
|
||||
|
||||
def test_known_tool_empty_list_args(self):
|
||||
"""C1/G1: Empty list arguments should not crash."""
|
||||
result = _hint([_tc("read_file", [])])
|
||||
assert result == "read_file"
|
||||
|
||||
def test_known_tool_none_args(self):
|
||||
"""G2: None arguments should not crash."""
|
||||
result = _hint([_tc("read_file", None)])
|
||||
assert result == "read_file"
|
||||
|
||||
def test_fallback_empty_list_args(self):
|
||||
"""C1: Empty list args in fallback should not crash."""
|
||||
result = _hint([_tc("custom_tool", [])])
|
||||
assert result == "custom_tool"
|
||||
|
||||
def test_fallback_none_args(self):
|
||||
"""G2: None args in fallback should not crash."""
|
||||
result = _hint([_tc("custom_tool", None)])
|
||||
assert result == "custom_tool"
|
||||
|
||||
def test_list_dir_registered(self):
|
||||
"""S2: list_dir should use 'ls' format."""
|
||||
result = _hint([_tc("list_dir", {"path": "/tmp"})])
|
||||
assert result == "ls /tmp"
|
||||
|
||||
|
||||
class TestToolHintMixedFolding:
|
||||
"""G4: Mixed folding groups with interleaved same-tool segments."""
|
||||
|
||||
def test_read_read_grep_grep_read(self):
|
||||
"""All different args — each hint listed separately."""
|
||||
calls = [
|
||||
_tc("read_file", {"path": "a.py"}),
|
||||
_tc("read_file", {"path": "b.py"}),
|
||||
_tc("grep", {"pattern": "x"}),
|
||||
_tc("grep", {"pattern": "y"}),
|
||||
_tc("read_file", {"path": "c.py"}),
|
||||
]
|
||||
result = _hint(calls)
|
||||
assert "\u00d7" not in result
|
||||
parts = result.split(", ")
|
||||
assert len(parts) == 5
|
||||
@@ -0,0 +1,502 @@
|
||||
"""Tests for unified_session feature.
|
||||
|
||||
Covers:
|
||||
- AgentLoop._dispatch() rewrites session_key to "unified:default" when enabled
|
||||
- Existing session_key_override is respected (not overwritten)
|
||||
- Feature is off by default (no behavior change for existing users)
|
||||
- Config schema serialises unified_session as camelCase "unifiedSession"
|
||||
- onboard-generated config.json contains "unifiedSession" key
|
||||
- /new command correctly clears the shared session in unified mode
|
||||
- /new is NOT a priority command (goes through _dispatch, key rewrite applies)
|
||||
- Context window consolidation is unaffected by unified_session
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.command.builtin import cmd_new, register_builtin_commands
|
||||
from nanobot.command.router import CommandContext, CommandRouter
|
||||
from nanobot.config.schema import AgentDefaults, Config
|
||||
from nanobot.session.manager import Session, SessionManager
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_loop(tmp_path: Path, unified_session: bool = False) -> AgentLoop:
|
||||
"""Create a minimal AgentLoop for dispatch-level tests."""
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
|
||||
with patch("nanobot.agent.loop.SessionManager"), \
|
||||
patch("nanobot.agent.loop.SubagentManager") as MockSubMgr, \
|
||||
patch("nanobot.agent.loop.Dream"):
|
||||
MockSubMgr.return_value.cancel_by_session = AsyncMock(return_value=0)
|
||||
loop = AgentLoop(
|
||||
bus=bus,
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
unified_session=unified_session,
|
||||
)
|
||||
return loop
|
||||
|
||||
|
||||
def _make_msg(channel: str = "telegram", chat_id: str = "111",
|
||||
session_key_override: str | None = None) -> InboundMessage:
|
||||
return InboundMessage(
|
||||
channel=channel,
|
||||
chat_id=chat_id,
|
||||
sender_id="user1",
|
||||
content="hello",
|
||||
session_key_override=session_key_override,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestUnifiedSessionDispatch — core behaviour
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestUnifiedSessionDispatch:
|
||||
"""AgentLoop._dispatch() session key rewriting logic."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_session_rewrites_key_to_unified_default(self, tmp_path: Path):
|
||||
"""When unified_session=True, all messages use 'unified:default' as session key."""
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
captured: list[str] = []
|
||||
|
||||
async def fake_process(msg, **kwargs):
|
||||
captured.append(msg.session_key)
|
||||
return None
|
||||
|
||||
loop._process_message = fake_process # type: ignore[method-assign]
|
||||
|
||||
msg = _make_msg(channel="telegram", chat_id="111")
|
||||
await loop._dispatch(msg)
|
||||
|
||||
assert captured == ["unified:default"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_session_different_channels_share_same_key(self, tmp_path: Path):
|
||||
"""Messages from different channels all resolve to the same session key."""
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
captured: list[str] = []
|
||||
|
||||
async def fake_process(msg, **kwargs):
|
||||
captured.append(msg.session_key)
|
||||
return None
|
||||
|
||||
loop._process_message = fake_process # type: ignore[method-assign]
|
||||
|
||||
await loop._dispatch(_make_msg(channel="telegram", chat_id="111"))
|
||||
await loop._dispatch(_make_msg(channel="discord", chat_id="222"))
|
||||
await loop._dispatch(_make_msg(channel="cli", chat_id="direct"))
|
||||
|
||||
assert captured == ["unified:default", "unified:default", "unified:default"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_session_disabled_preserves_original_key(self, tmp_path: Path):
|
||||
"""When unified_session=False (default), session key is channel:chat_id as usual."""
|
||||
loop = _make_loop(tmp_path, unified_session=False)
|
||||
|
||||
captured: list[str] = []
|
||||
|
||||
async def fake_process(msg, **kwargs):
|
||||
captured.append(msg.session_key)
|
||||
return None
|
||||
|
||||
loop._process_message = fake_process # type: ignore[method-assign]
|
||||
|
||||
msg = _make_msg(channel="telegram", chat_id="999")
|
||||
await loop._dispatch(msg)
|
||||
|
||||
assert captured == ["telegram:999"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_session_respects_existing_override(self, tmp_path: Path):
|
||||
"""If session_key_override is already set (e.g. Telegram thread), it is NOT overwritten."""
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
captured: list[str] = []
|
||||
|
||||
async def fake_process(msg, **kwargs):
|
||||
captured.append(msg.session_key)
|
||||
return None
|
||||
|
||||
loop._process_message = fake_process # type: ignore[method-assign]
|
||||
|
||||
msg = _make_msg(channel="telegram", chat_id="111", session_key_override="telegram:thread:42")
|
||||
await loop._dispatch(msg)
|
||||
|
||||
assert captured == ["telegram:thread:42"]
|
||||
|
||||
def test_unified_session_default_is_false(self, tmp_path: Path):
|
||||
"""unified_session defaults to False — no behavior change for existing users."""
|
||||
loop = _make_loop(tmp_path)
|
||||
assert loop._unified_session is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestUnifiedSessionConfig — schema & serialisation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestUnifiedSessionConfig:
|
||||
"""Config schema and onboard serialisation for unified_session."""
|
||||
|
||||
def test_agent_defaults_unified_session_default_is_false(self):
|
||||
"""AgentDefaults.unified_session defaults to False."""
|
||||
defaults = AgentDefaults()
|
||||
assert defaults.unified_session is False
|
||||
|
||||
def test_agent_defaults_unified_session_can_be_enabled(self):
|
||||
"""AgentDefaults.unified_session can be set to True."""
|
||||
defaults = AgentDefaults(unified_session=True)
|
||||
assert defaults.unified_session is True
|
||||
|
||||
def test_config_serialises_unified_session_as_camel_case(self):
|
||||
"""model_dump(by_alias=True) outputs 'unifiedSession' (camelCase) for JSON."""
|
||||
config = Config()
|
||||
data = config.model_dump(mode="json", by_alias=True)
|
||||
agents_defaults = data["agents"]["defaults"]
|
||||
assert "unifiedSession" in agents_defaults
|
||||
assert agents_defaults["unifiedSession"] is False
|
||||
|
||||
def test_config_parses_unified_session_from_camel_case(self):
|
||||
"""Config can be loaded from JSON with camelCase 'unifiedSession'."""
|
||||
raw = {"agents": {"defaults": {"unifiedSession": True}}}
|
||||
config = Config.model_validate(raw)
|
||||
assert config.agents.defaults.unified_session is True
|
||||
|
||||
def test_config_parses_unified_session_from_snake_case(self):
|
||||
"""Config also accepts snake_case 'unified_session' (populate_by_name=True)."""
|
||||
raw = {"agents": {"defaults": {"unified_session": True}}}
|
||||
config = Config.model_validate(raw)
|
||||
assert config.agents.defaults.unified_session is True
|
||||
|
||||
def test_onboard_generated_config_contains_unified_session(self, tmp_path: Path):
|
||||
"""save_config() writes 'unifiedSession' into config.json (simulates nanobot onboard)."""
|
||||
from nanobot.config.loader import save_config
|
||||
|
||||
config = Config()
|
||||
config_path = tmp_path / "config.json"
|
||||
save_config(config, config_path)
|
||||
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
data = json.load(f)
|
||||
|
||||
agents_defaults = data["agents"]["defaults"]
|
||||
assert "unifiedSession" in agents_defaults, (
|
||||
"onboard-generated config.json must contain 'unifiedSession' key"
|
||||
)
|
||||
assert agents_defaults["unifiedSession"] is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestCmdNewUnifiedSession — /new command behaviour in unified mode
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCmdNewUnifiedSession:
|
||||
"""/new command routing and session-clear behaviour in unified mode."""
|
||||
|
||||
def test_new_is_not_a_priority_command(self):
|
||||
"""/new must NOT be in the priority table — it must go through _dispatch()
|
||||
so the unified session key rewrite applies before cmd_new runs."""
|
||||
router = CommandRouter()
|
||||
register_builtin_commands(router)
|
||||
assert router.is_priority("/new") is False
|
||||
|
||||
def test_new_is_an_exact_command(self):
|
||||
"""/new must be registered as an exact command."""
|
||||
router = CommandRouter()
|
||||
register_builtin_commands(router)
|
||||
assert "/new" in router._exact
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cmd_new_clears_unified_session(self, tmp_path: Path):
|
||||
"""cmd_new called with key='unified:default' clears the shared session."""
|
||||
sessions = SessionManager(tmp_path)
|
||||
|
||||
# Pre-populate the shared session with some messages
|
||||
shared = sessions.get_or_create("unified:default")
|
||||
shared.add_message("user", "hello from telegram")
|
||||
shared.add_message("assistant", "hi there")
|
||||
sessions.save(shared)
|
||||
assert len(sessions.get_or_create("unified:default").messages) == 2
|
||||
|
||||
# _schedule_background is a *sync* method that schedules a coroutine via
|
||||
# asyncio.create_task(). Mirror that exactly so the coroutine is consumed
|
||||
# and no RuntimeWarning is emitted.
|
||||
loop = SimpleNamespace(
|
||||
sessions=sessions,
|
||||
consolidator=SimpleNamespace(archive=AsyncMock(return_value=True)),
|
||||
)
|
||||
loop._schedule_background = lambda coro: asyncio.ensure_future(coro)
|
||||
|
||||
msg = InboundMessage(
|
||||
channel="telegram", sender_id="user1", chat_id="111", content="/new",
|
||||
session_key_override="unified:default", # as _dispatch() would set it
|
||||
)
|
||||
ctx = CommandContext(msg=msg, session=None, key="unified:default", raw="/new", loop=loop)
|
||||
|
||||
result = await cmd_new(ctx)
|
||||
|
||||
assert "New session started" in result.content
|
||||
# Invalidate cache and reload from disk to confirm persistence
|
||||
sessions.invalidate("unified:default")
|
||||
reloaded = sessions.get_or_create("unified:default")
|
||||
assert reloaded.messages == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cmd_new_in_unified_mode_does_not_affect_other_sessions(self, tmp_path: Path):
|
||||
"""Clearing unified:default must not touch other sessions on disk."""
|
||||
sessions = SessionManager(tmp_path)
|
||||
|
||||
other = sessions.get_or_create("discord:999")
|
||||
other.add_message("user", "discord message")
|
||||
sessions.save(other)
|
||||
|
||||
shared = sessions.get_or_create("unified:default")
|
||||
shared.add_message("user", "shared message")
|
||||
sessions.save(shared)
|
||||
|
||||
loop = SimpleNamespace(
|
||||
sessions=sessions,
|
||||
consolidator=SimpleNamespace(archive=AsyncMock(return_value=True)),
|
||||
)
|
||||
loop._schedule_background = lambda coro: asyncio.ensure_future(coro)
|
||||
|
||||
msg = InboundMessage(
|
||||
channel="telegram", sender_id="user1", chat_id="111", content="/new",
|
||||
session_key_override="unified:default",
|
||||
)
|
||||
ctx = CommandContext(msg=msg, session=None, key="unified:default", raw="/new", loop=loop)
|
||||
await cmd_new(ctx)
|
||||
|
||||
sessions.invalidate("unified:default")
|
||||
sessions.invalidate("discord:999")
|
||||
assert sessions.get_or_create("unified:default").messages == []
|
||||
assert len(sessions.get_or_create("discord:999").messages) == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestConsolidationUnaffectedByUnifiedSession — consolidation is key-agnostic
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestConsolidationUnaffectedByUnifiedSession:
|
||||
"""maybe_consolidate_by_tokens() behaviour is identical regardless of session key."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_consolidation_skips_empty_session_for_unified_key(self):
|
||||
"""Empty unified:default session → consolidation exits immediately, archive not called."""
|
||||
from nanobot.agent.memory import Consolidator, MemoryStore
|
||||
|
||||
store = MagicMock(spec=MemoryStore)
|
||||
mock_provider = MagicMock()
|
||||
mock_provider.chat_with_retry = AsyncMock(return_value=MagicMock(content="summary"))
|
||||
# Use spec= so MagicMock doesn't auto-generate AsyncMock for non-async methods,
|
||||
# which would leave unawaited coroutines and trigger RuntimeWarning.
|
||||
sessions = MagicMock(spec=SessionManager)
|
||||
|
||||
consolidator = Consolidator(
|
||||
store=store,
|
||||
provider=mock_provider,
|
||||
model="test-model",
|
||||
sessions=sessions,
|
||||
context_window_tokens=1000,
|
||||
build_messages=MagicMock(return_value=[]),
|
||||
get_tool_definitions=MagicMock(return_value=[]),
|
||||
max_completion_tokens=100,
|
||||
)
|
||||
consolidator.archive = AsyncMock()
|
||||
|
||||
session = Session(key="unified:default")
|
||||
session.messages = []
|
||||
|
||||
await consolidator.maybe_consolidate_by_tokens(session)
|
||||
|
||||
consolidator.archive.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_consolidation_behaviour_identical_for_any_key(self):
|
||||
"""archive call count is the same for 'telegram:123' and 'unified:default'
|
||||
under identical token conditions."""
|
||||
from nanobot.agent.memory import Consolidator, MemoryStore
|
||||
|
||||
archive_calls: dict[str, int] = {}
|
||||
|
||||
for key in ("telegram:123", "unified:default"):
|
||||
store = MagicMock(spec=MemoryStore)
|
||||
mock_provider = MagicMock()
|
||||
mock_provider.chat_with_retry = AsyncMock(return_value=MagicMock(content="summary"))
|
||||
sessions = MagicMock(spec=SessionManager)
|
||||
|
||||
consolidator = Consolidator(
|
||||
store=store,
|
||||
provider=mock_provider,
|
||||
model="test-model",
|
||||
sessions=sessions,
|
||||
context_window_tokens=1000,
|
||||
build_messages=MagicMock(return_value=[]),
|
||||
get_tool_definitions=MagicMock(return_value=[]),
|
||||
max_completion_tokens=100,
|
||||
)
|
||||
|
||||
session = Session(key=key)
|
||||
session.messages = [] # empty → exits immediately for both keys
|
||||
|
||||
consolidator.archive = AsyncMock()
|
||||
await consolidator.maybe_consolidate_by_tokens(session)
|
||||
archive_calls[key] = consolidator.archive.call_count
|
||||
|
||||
assert archive_calls["telegram:123"] == archive_calls["unified:default"] == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_consolidation_triggers_when_over_budget_unified_key(self):
|
||||
"""When tokens exceed budget, consolidation attempts to find a boundary —
|
||||
behaviour is identical to any other session key."""
|
||||
from nanobot.agent.memory import Consolidator, MemoryStore
|
||||
|
||||
store = MagicMock(spec=MemoryStore)
|
||||
mock_provider = MagicMock()
|
||||
sessions = MagicMock(spec=SessionManager)
|
||||
|
||||
consolidator = Consolidator(
|
||||
store=store,
|
||||
provider=mock_provider,
|
||||
model="test-model",
|
||||
sessions=sessions,
|
||||
context_window_tokens=1000,
|
||||
build_messages=MagicMock(return_value=[]),
|
||||
get_tool_definitions=MagicMock(return_value=[]),
|
||||
max_completion_tokens=100,
|
||||
)
|
||||
|
||||
session = Session(key="unified:default")
|
||||
session.messages = [{"role": "user", "content": "msg"}]
|
||||
|
||||
# Simulate over-budget: estimated > budget
|
||||
consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(950, "tiktoken"))
|
||||
# No valid boundary found → returns gracefully without archiving
|
||||
consolidator.pick_consolidation_boundary = MagicMock(return_value=None)
|
||||
consolidator.archive = AsyncMock()
|
||||
|
||||
await consolidator.maybe_consolidate_by_tokens(session)
|
||||
|
||||
# estimate was called (consolidation was attempted)
|
||||
consolidator.estimate_session_prompt_tokens.assert_called_once_with(session)
|
||||
# but archive was not called (no valid boundary)
|
||||
consolidator.archive.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestStopCommandWithUnifiedSession — /stop command integration
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestStopCommandWithUnifiedSession:
|
||||
"""Verify /stop command works correctly with unified session enabled."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_active_tasks_use_effective_key_in_unified_mode(self, tmp_path: Path):
|
||||
"""When unified_session=True, tasks are stored under UNIFIED_SESSION_KEY."""
|
||||
from nanobot.agent.loop import UNIFIED_SESSION_KEY
|
||||
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
# Create a message from telegram channel
|
||||
msg = _make_msg(channel="telegram", chat_id="123456")
|
||||
|
||||
# Mock _dispatch to complete immediately
|
||||
async def fake_dispatch(m):
|
||||
pass
|
||||
|
||||
loop._dispatch = fake_dispatch # type: ignore[method-assign]
|
||||
|
||||
# Simulate the task creation flow (from _run loop)
|
||||
effective_key = UNIFIED_SESSION_KEY if loop._unified_session and not msg.session_key_override else msg.session_key
|
||||
task = asyncio.create_task(loop._dispatch(msg))
|
||||
loop._active_tasks.setdefault(effective_key, []).append(task)
|
||||
|
||||
# Wait for task to complete
|
||||
await task
|
||||
|
||||
# Verify the task is stored under UNIFIED_SESSION_KEY, not the original channel:chat_id
|
||||
assert UNIFIED_SESSION_KEY in loop._active_tasks
|
||||
assert "telegram:123456" not in loop._active_tasks
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_command_finds_task_in_unified_mode(self, tmp_path: Path):
|
||||
"""cmd_stop can cancel tasks when unified_session=True."""
|
||||
from nanobot.agent.loop import UNIFIED_SESSION_KEY
|
||||
from nanobot.command.builtin import cmd_stop
|
||||
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
# Create a long-running task stored under UNIFIED_SESSION_KEY
|
||||
async def long_running():
|
||||
await asyncio.sleep(10) # Will be cancelled
|
||||
|
||||
task = asyncio.create_task(long_running())
|
||||
loop._active_tasks[UNIFIED_SESSION_KEY] = [task]
|
||||
|
||||
# Create a message that would have session_key=UNIFIED_SESSION_KEY after dispatch
|
||||
msg = InboundMessage(
|
||||
channel="telegram",
|
||||
chat_id="123456",
|
||||
sender_id="user1",
|
||||
content="/stop",
|
||||
session_key_override=UNIFIED_SESSION_KEY, # Simulate post-dispatch state
|
||||
)
|
||||
|
||||
ctx = CommandContext(msg=msg, session=None, key=UNIFIED_SESSION_KEY, raw="/stop", loop=loop)
|
||||
|
||||
# Execute /stop
|
||||
result = await cmd_stop(ctx)
|
||||
|
||||
# Verify task was cancelled
|
||||
assert task.cancelled() or task.done()
|
||||
assert "Stopped 1 task" in result.content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_command_cross_channel_in_unified_mode(self, tmp_path: Path):
|
||||
"""In unified mode, /stop from one channel cancels tasks from another channel."""
|
||||
from nanobot.agent.loop import UNIFIED_SESSION_KEY
|
||||
from nanobot.command.builtin import cmd_stop
|
||||
|
||||
loop = _make_loop(tmp_path, unified_session=True)
|
||||
|
||||
# Create tasks from different channels, all stored under UNIFIED_SESSION_KEY
|
||||
async def long_running():
|
||||
await asyncio.sleep(10)
|
||||
|
||||
task1 = asyncio.create_task(long_running())
|
||||
task2 = asyncio.create_task(long_running())
|
||||
loop._active_tasks[UNIFIED_SESSION_KEY] = [task1, task2]
|
||||
|
||||
# /stop from discord should cancel tasks started from telegram
|
||||
msg = InboundMessage(
|
||||
channel="discord",
|
||||
chat_id="789012",
|
||||
sender_id="user2",
|
||||
content="/stop",
|
||||
session_key_override=UNIFIED_SESSION_KEY,
|
||||
)
|
||||
|
||||
ctx = CommandContext(msg=msg, session=None, key=UNIFIED_SESSION_KEY, raw="/stop", loop=loop)
|
||||
|
||||
result = await cmd_stop(ctx)
|
||||
|
||||
# Both tasks should be cancelled
|
||||
assert "Stopped 2 task" in result.content
|
||||
Reference in New Issue
Block a user