From 97d51ca20db1b0d89ae69cb2a3b3cd78ea7389cf Mon Sep 17 00:00:00 2001 From: kshitij Date: Thu, 23 Jul 2026 17:13:46 +0500 Subject: [PATCH] fix: use canonical _get_platform_tools resolver for memory tool status The PR's inline toolset resolution (checking 'memory' in cli_toolsets list) produced wrong results for composite toolsets like 'hermes-cli' which expand to include the memory tool. Replace with the canonical _get_platform_tools() from tools_config.py which correctly handles composite toolsets and all edge cases. Update tests to mock _get_platform_tools instead of raw config. --- hermes_cli/memory_setup.py | 14 +++++------- tests/hermes_cli/test_memory_status.py | 31 +++++++++++++++++--------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index 19d18d602366..e08f61c5d348 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -428,15 +428,11 @@ def cmd_status(args) -> None: mem_mark = "enabled ✓" if memory_enabled else "disabled ✗" user_mark = "enabled ✓" if user_profile_enabled else "disabled ✗" - # Check if the memory toolset is enabled for the CLI platform - # platform_toolsets.cli is either None (default = all enabled) or - # an explicit list of enabled toolset names. - platform_toolsets = config.get("platform_toolsets", {}) or {} - cli_toolsets = platform_toolsets.get("cli") - memory_tool_enabled = ( - cli_toolsets is None - or (isinstance(cli_toolsets, list) and "memory" in cli_toolsets) - ) + # Check if the memory tool is enabled for the CLI platform via the + # canonical resolver (handles composite toolsets like hermes-cli). + from hermes_cli.tools_config import _get_platform_tools + cli_tools = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + memory_tool_enabled = "memory" in cli_tools tool_mark = "enabled ✓" if memory_tool_enabled else "disabled ✗" print("\nMemory status\n" + "─" * 40) diff --git a/tests/hermes_cli/test_memory_status.py b/tests/hermes_cli/test_memory_status.py index ba42f237654b..e5242c5d4b93 100644 --- a/tests/hermes_cli/test_memory_status.py +++ b/tests/hermes_cli/test_memory_status.py @@ -2,7 +2,8 @@ Covers: - Status output shows config-aware indicators instead of hardcoded 'always active' -- memory_enabled, user_profile_enabled, and memory toolset are each reflected +- memory_enabled, user_profile_enabled, and memory tool are each reflected +- Memory tool resolution uses the canonical _get_platform_tools resolver - Original issue: 'Built-in: always active' was misleading when features were disabled """ @@ -10,17 +11,27 @@ import pytest from unittest.mock import patch -def _run_cmd_status(capfd, mem_config=None, platform_toolsets=None): - """Run cmd_status with a mocked config and return captured stdout.""" +def _run_cmd_status(capfd, mem_config=None, memory_tools=None): + """Run cmd_status with a mocked config and return captured stdout. + + Args: + mem_config: The "memory" section of config. + memory_tools: Set of tool names returned by _get_platform_tools. + Defaults to {"memory"} (tool enabled). + """ from hermes_cli.memory_setup import cmd_status config = {"memory": mem_config or {}} - if platform_toolsets is not None: - config["platform_toolsets"] = platform_toolsets + if memory_tools is None: + memory_tools = {"memory"} with patch("hermes_cli.config.load_config", return_value=config): with patch("hermes_cli.memory_setup._get_available_providers", return_value=[]): - cmd_status(args=None) + with patch( + "hermes_cli.tools_config._get_platform_tools", + return_value=memory_tools, + ): + cmd_status(args=None) captured = capfd.readouterr() return captured.out @@ -63,7 +74,7 @@ class TestMemoryStatusLabels: assert "enabled ✓" in out def test_memory_tool_enabled_by_default(self, capfd): - """Memory tool is enabled by default (no platform_toolsets = all enabled).""" + """Memory tool is enabled by default.""" out = _run_cmd_status(capfd) assert "Memory tool:" in out assert "enabled ✓" in out @@ -72,7 +83,7 @@ class TestMemoryStatusLabels: """When CLI toolset excludes 'memory', the tool shows disabled.""" out = _run_cmd_status( capfd, - platform_toolsets={"cli": ["terminal", "file"]}, + memory_tools={"terminal", "file"}, ) assert "Memory tool:" in out assert "disabled ✗" in out @@ -81,7 +92,7 @@ class TestMemoryStatusLabels: """When CLI toolset includes 'memory', the tool shows enabled.""" out = _run_cmd_status( capfd, - platform_toolsets={"cli": ["terminal", "file", "memory"]}, + memory_tools={"terminal", "file", "memory"}, ) assert "Memory tool:" in out assert "enabled ✓" in out @@ -99,7 +110,7 @@ class TestMemoryStatusLabels: out = _run_cmd_status( capfd, mem_config={"memory_enabled": False, "user_profile_enabled": False}, - platform_toolsets={"cli": ["terminal"]}, + memory_tools=set(), ) assert out.count("disabled ✗") == 3 assert "always active" not in out