diff --git a/tests/agent/test_builtin_memory_disabled_surface.py b/tests/agent/test_builtin_memory_disabled_surface.py new file mode 100644 index 0000000000..d2771bf463 --- /dev/null +++ b/tests/agent/test_builtin_memory_disabled_surface.py @@ -0,0 +1,161 @@ +"""Built-in memory disabled in config must leave no dead surface behind. + +Setting ``memory.memory_enabled: false`` and ``memory.user_profile_enabled: +false`` stops ``agent_init`` from building a ``MemoryStore``, so the ``memory`` +tool dispatches against ``store=None`` and every call comes back "Memory is not +available". Before the fix the tool stayed in the schema and MEMORY_GUIDANCE +stayed in the system prompt, so users running a third-party provider (Hindsight, +Mem0, …) paid for both on every API call with no way to drop them — listing +``memory`` under ``disabled_toolsets`` takes the provider's tools down too. + +These tests exercise the real resolution chain (config on disk → check_fn → +``get_tool_definitions``) against a temp ``HERMES_HOME``, not mocks. +""" + +import pytest +import yaml + +from model_tools import get_tool_definitions + + +@pytest.fixture(autouse=True) +def _clear_caches(): + """check_fn results and tool definitions are both cached; config written by + a test only takes effect once those are dropped.""" + from model_tools import _clear_tool_defs_cache + from tools.registry import invalidate_check_fn_cache + + invalidate_check_fn_cache() + _clear_tool_defs_cache() + yield + invalidate_check_fn_cache() + _clear_tool_defs_cache() + + +def _write_memory_config(home, **memory_section): + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text( + yaml.safe_dump({"memory": memory_section}), encoding="utf-8" + ) + + +@pytest.fixture +def hermes_home(tmp_path, monkeypatch): + home = tmp_path / ".hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + return home + + +def _memory_tool_names(): + tools = get_tool_definitions(enabled_toolsets=["memory"], quiet_mode=True) + return {tool["function"]["name"] for tool in tools} + + +class TestBuiltinMemoryToolAvailability: + def test_tool_hidden_when_both_stores_disabled(self, hermes_home): + _write_memory_config( + hermes_home, memory_enabled=False, user_profile_enabled=False + ) + assert "memory" not in _memory_tool_names() + + def test_tool_present_when_only_user_profile_enabled(self, hermes_home): + _write_memory_config( + hermes_home, memory_enabled=False, user_profile_enabled=True + ) + assert "memory" in _memory_tool_names() + + def test_tool_present_when_only_memory_enabled(self, hermes_home): + _write_memory_config( + hermes_home, memory_enabled=True, user_profile_enabled=False + ) + assert "memory" in _memory_tool_names() + + def test_tool_present_by_default(self, hermes_home): + """No config file at all must not strip a working tool.""" + assert "memory" in _memory_tool_names() + + def test_unreadable_config_fails_open(self, hermes_home, monkeypatch): + """A config read error must not silently remove the tool.""" + from tools import memory_tool as memory_tool_module + + def _boom(): + raise RuntimeError("config unreadable") + + monkeypatch.setattr( + "hermes_cli.config.load_config_readonly", _boom, raising=False + ) + assert memory_tool_module.check_memory_requirements() is True + + +class TestExternalProviderSurvivesBuiltinDisable: + """Dropping the built-in tool must not drop the external provider's tools. + + ``memory_provider_tools_enabled`` short-circuits on the built-in tool being + present, so hiding that tool moves the decision onto the toolset gate. The + provider must still be reachable for every way a caller can ask for memory. + """ + + def test_provider_tools_enabled_when_memory_toolset_requested(self): + from agent.memory_manager import memory_provider_tools_enabled + + assert memory_provider_tools_enabled( + ["memory", "file"], None, memory_tool_present=False + ) + + def test_provider_tools_enabled_for_unrestricted_toolsets(self): + from agent.memory_manager import memory_provider_tools_enabled + + assert memory_provider_tools_enabled(None, None, memory_tool_present=False) + + def test_disabled_toolsets_still_takes_everything_down(self): + """The heavy switch keeps its documented meaning.""" + from agent.memory_manager import memory_provider_tools_enabled + + assert not memory_provider_tools_enabled( + None, ["memory"], memory_tool_present=False + ) + + +class TestInjectionEndToEnd: + """The real ``inject_memory_provider_tools`` with no built-in memory tool.""" + + def test_provider_tools_injected_without_builtin_memory_tool(self): + from types import SimpleNamespace + + from agent.memory_manager import MemoryManager, inject_memory_provider_tools + from agent.memory_provider import MemoryProvider + + class _Provider(MemoryProvider): + @property + def name(self): + return "fake_hindsight" + + def is_available(self): + return True + + def initialize(self, session_id, **kwargs): + pass + + def get_tool_schemas(self): + return [ + { + "name": "hindsight_retain", + "description": "retain", + "parameters": {"type": "object", "properties": {}}, + } + ] + + manager = MemoryManager() + manager.add_provider(_Provider()) + agent = SimpleNamespace( + _memory_manager=manager, + enabled_toolsets=["memory"], + disabled_toolsets=None, + tools=[], + valid_tool_names=set(), + ) + + added = inject_memory_provider_tools(agent) + + assert added == 1 + assert "hindsight_retain" in agent.valid_tool_names diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index cd4836c575..8713b588fa 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -910,6 +910,35 @@ class TestBuildSystemPrompt: def test_memory_guidance_when_memory_tool_loaded(self, agent_with_memory_tool): from agent.prompt_builder import MEMORY_GUIDANCE + agent_with_memory_tool._memory_enabled = True + prompt = agent_with_memory_tool._build_system_prompt() + assert MEMORY_GUIDANCE in prompt + + def test_no_memory_guidance_when_both_builtin_stores_disabled( + self, agent_with_memory_tool + ): + """Guidance must follow the stores, not just the tool's presence. + + With both built-in stores off, ``agent_init`` never builds a + ``MemoryStore``, so every memory call returns "Memory is not + available" — telling the model to save facts there is a dead + instruction paid for on every API call. + """ + from agent.prompt_builder import MEMORY_GUIDANCE + + agent_with_memory_tool._memory_enabled = False + agent_with_memory_tool._user_profile_enabled = False + prompt = agent_with_memory_tool._build_system_prompt() + assert MEMORY_GUIDANCE not in prompt + + def test_memory_guidance_when_only_user_profile_enabled( + self, agent_with_memory_tool + ): + """USER.md alone still backs the tool, so the guidance stays.""" + from agent.prompt_builder import MEMORY_GUIDANCE + + agent_with_memory_tool._memory_enabled = False + agent_with_memory_tool._user_profile_enabled = True prompt = agent_with_memory_tool._build_system_prompt() assert MEMORY_GUIDANCE in prompt