From 387b5c3604f07edfda276c84897461c16def5f43 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Thu, 20 Aug 2026 08:19:32 +0700 Subject: [PATCH 1/2] fix(memory): drop dead memory tool and guidance when built-in stores are off With memory.memory_enabled and memory.user_profile_enabled both false, agent_init never builds a MemoryStore -- but check_memory_requirements() returned True unconditionally and MEMORY_GUIDANCE was gated only on the tool being present in valid_tool_names. So the tool shipped in every request's schema while answering "Memory is not available" on every call, and the system prompt still told the model to save durable facts there. Gate both on the config flags, using the store predicate for the tool and the already-resolved agent state for the guidance (config is not re-read mid-conversation, so the prompt stays byte-stable). Either flag alone still backs the tool, so only turning both off removes it. This lets a user running a third-party provider (Hindsight, Mem0, ...) turn the built-in files off without paying for the dead surface on every API call. The provider's own tools are unaffected: hiding the built-in tool moves the decision onto the toolset gate, and listing memory under agent.disabled_toolsets remains the only switch that takes those down. --- agent/system_prompt.py | 10 +++++++- tools/memory_tool.py | 29 ++++++++++++++++++++-- website/docs/user-guide/features/memory.md | 9 +++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/agent/system_prompt.py b/agent/system_prompt.py index 7d5bba76b3b3e..64399226d5baa 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -415,7 +415,15 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # Tool-aware behavioral guidance: only inject when the tools are loaded tool_guidance = [] - if "memory" in agent.valid_tool_names: + # MEMORY_GUIDANCE instructs the model to save facts to the built-in + # MEMORY.md/USER.md stores. With both disabled in config no store is built, + # so the guidance would steer the model at a tool whose every call returns + # "Memory is not available". Defaults to True for the rare code paths that + # build an agent view without going through agent_init. + builtin_memory_active = getattr(agent, "_memory_enabled", True) or getattr( + agent, "_user_profile_enabled", True + ) + if "memory" in agent.valid_tool_names and builtin_memory_active: tool_guidance.append(MEMORY_GUIDANCE) if "session_search" in agent.valid_tool_names: tool_guidance.append(SESSION_SEARCH_GUIDANCE) diff --git a/tools/memory_tool.py b/tools/memory_tool.py index 44effd02c27e4..c2ba72dc0ed22 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -1143,9 +1143,34 @@ def memory_tool( return json.dumps(result, ensure_ascii=False) +def builtin_memory_stores_enabled() -> bool: + """Return whether either built-in store (MEMORY.md / USER.md) is enabled. + + ``agent_init`` only builds a ``MemoryStore`` when at least one of + ``memory.memory_enabled`` / ``memory.user_profile_enabled`` is true, so with + both off the tool dispatches against ``store=None`` and every call fails + with "Memory is not available". + + Fails open when config can't be read: an unreadable config must not strip a + tool that would otherwise work. + """ + try: + from hermes_cli.config import load_config_readonly + + section = (load_config_readonly() or {}).get("memory") + if not isinstance(section, dict): + return True + return bool(section.get("memory_enabled", True)) or bool( + section.get("user_profile_enabled", True) + ) + except Exception: + logger.debug("Could not read memory config for availability", exc_info=True) + return True + + def check_memory_requirements() -> bool: - """Memory tool has no external requirements -- always available.""" - return True + """Available unless both built-in memory stores are disabled in config.""" + return builtin_memory_stores_enabled() def apply_memory_pending(payload: Dict[str, Any], store: "MemoryStore") -> Dict[str, Any]: diff --git a/website/docs/user-guide/features/memory.md b/website/docs/user-guide/features/memory.md index 624465821e386..f76863da65d82 100644 --- a/website/docs/user-guide/features/memory.md +++ b/website/docs/user-guide/features/memory.md @@ -240,6 +240,15 @@ memory: write_approval: false # false = write freely (default) | true = require approval ``` +Setting **both** `memory_enabled` and `user_profile_enabled` to `false` turns the +built-in stores off completely: the `memory` tool is dropped from the schema and +its guidance block is dropped from the system prompt, so the model is never told +about a tool it cannot use. An external provider set via `memory.provider` +(Hindsight, Mem0, Honcho, …) is unaffected and keeps its own tools — use this +when you want a third-party memory backend *instead of* the built-in files. +Listing `memory` under `agent.disabled_toolsets` is the heavier switch: it hides +external provider tools too. + ## Controlling memory writes (`write_approval`) By default the agent saves memory freely — including from the background From 38934c79f9b3e5fcad4c4d5f68967860e207e984 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Thu, 20 Aug 2026 08:19:43 +0700 Subject: [PATCH 2/2] test(memory): cover the disabled built-in memory surface Walks the real resolution chain -- config.yaml on a temp HERMES_HOME -> check_memory_requirements -> get_tool_definitions -- rather than mocking the availability check, since the bug was in how the flags reach the schema. Covers both flags off, either one alone, no config file at all, and a config read that raises (must fail open). Also asserts the external provider's tools survive with the built-in tool gone, so the fix cannot regress into taking Hindsight/Mem0 down with it, while disabled_toolsets keeps its documented "hide everything" meaning. The existing MEMORY_GUIDANCE test built a skip_memory agent whose flags were both false, so it was asserting the old tool-presence-only behavior; it now states its precondition and gains the false-case mirror. --- .../test_builtin_memory_disabled_surface.py | 161 ++++++++++++++++++ tests/run_agent/test_run_agent.py | 29 ++++ 2 files changed, 190 insertions(+) create mode 100644 tests/agent/test_builtin_memory_disabled_surface.py 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 0000000000000..d2771bf463886 --- /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 cd4836c575f28..8713b588fa3c6 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -910,6 +910,35 @@ def test_can_use_soul_identity_even_when_context_files_are_skipped(self): 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