From 3e16176ba46f195263179386c708df9b353dcfda Mon Sep 17 00:00:00 2001 From: ygd58 Date: Sun, 21 Jun 2026 08:52:49 +0200 Subject: [PATCH] fix(tools): reconcile agent.disabled_toolsets when a toolset is enabled _get_platform_tools() applies agent.disabled_toolsets as a final override AFTER reading platform_toolsets., so a toolset listed there stays permanently OFF no matter what the toggle write path saves. Blank Slate installs pre-populate this list with ~27 toolsets, making most of the desktop Toolsets UI un-enableable (issue #49995). Fix: _save_platform_tools() now removes any toolset the user just explicitly enabled FOR THIS PLATFORM from agent.disabled_toolsets. Toolsets the user did not touch, or that remain disabled on other platforms, are left alone -- disabled_toolsets keeps working as a cross-platform suppression list for anything not actively re-enabled. Disabling a toolset (unchecking it) does not touch disabled_toolsets at all -- only enables reconcile it. Verified end-to-end with the exact repro from the issue: Blank Slate config (disabled_toolsets=['todo','memory','browser'], cli=['file', 'terminal']) -> enable 'todo' via the toggle -> _get_platform_tools() now resolves 'todo' as enabled while 'memory'/'browser' (untouched) remain disabled. Added 4 regression tests. Full tools_config suite: 101 passed (97 existing + 4 new), no regressions. Fixes #49995 --- hermes_cli/tools_config.py | 26 +++++++++ tests/hermes_cli/test_tools_config.py | 80 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 3dc4cecce83..d0b024b7449 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -1736,6 +1736,32 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[ config.setdefault("known_plugin_toolsets", {}) config["known_plugin_toolsets"][platform] = sorted(plugin_keys) + # Reconcile with agent.disabled_toolsets. _get_platform_tools() applies + # that list as a final override AFTER reading platform_toolsets., + # so a toolset listed there stays permanently OFF no matter what this + # function writes — the toggle "saves" but silently can't ever take + # effect. Blank Slate installs pre-populate this list with ~27 toolsets, + # making most of the desktop Toolsets UI unusable for re-enabling + # anything (issue #49995). + # + # Only toolsets the user just explicitly enabled FOR THIS PLATFORM are + # cleared from the global disabled list — toolsets the user did not + # touch (still unchecked) or that remain disabled on other platforms + # are left alone, so agent.disabled_toolsets keeps working as a + # cross-platform suppression list for anything not actively re-enabled. + agent_cfg = config.get("agent") + if isinstance(agent_cfg, dict): + disabled_toolsets = agent_cfg.get("disabled_toolsets") + if isinstance(disabled_toolsets, list) and disabled_toolsets: + newly_enabled = enabled_toolset_keys - preserved_entries + if newly_enabled: + remaining = [ + ts for ts in disabled_toolsets + if str(ts) not in newly_enabled + ] + if remaining != disabled_toolsets: + agent_cfg["disabled_toolsets"] = remaining + save_config(config) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 4280ab183af..dbe95a0cb5d 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -1676,3 +1676,83 @@ def test_vision_picker_custom_endpoint(tmp_path, monkeypatch): save_env.assert_called_once_with("OPENAI_API_KEY", "sk-secret") +def test_save_platform_tools_clears_newly_enabled_from_disabled_toolsets(): + """Enabling a toolset via the picker must remove it from + agent.disabled_toolsets, or _get_platform_tools() permanently masks it + back to OFF on the next read no matter what platform_toolsets says. + + Blank Slate installs pre-populate disabled_toolsets with ~27 toolsets, + making the desktop Toolsets UI's enable toggle effectively a no-op for + any of them (issue #49995). + """ + config = { + "platform_toolsets": {"cli": ["file", "terminal"]}, + "agent": {"disabled_toolsets": ["todo", "memory", "browser"]}, + } + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"file", "terminal", "todo"}) + + # The toolset the user just enabled is cleared from the block-list... + assert "todo" not in config["agent"]["disabled_toolsets"] + # ...but toolsets the user did NOT touch stay disabled (no over-reach). + assert "memory" in config["agent"]["disabled_toolsets"] + assert "browser" in config["agent"]["disabled_toolsets"] + assert "todo" in config["platform_toolsets"]["cli"] + + +def test_save_platform_tools_resolves_to_enabled_after_disabled_toolsets_reconcile(): + """End-to-end: after _save_platform_tools() reconciles disabled_toolsets, + _get_platform_tools() must actually resolve the toolset as enabled -- + this is the exact symptom from issue #49995 (toggle saves but the UI/agent + still reads it as OFF after reopen). + """ + config = { + "platform_toolsets": {"cli": ["file", "terminal"]}, + "agent": {"disabled_toolsets": ["todo", "memory"]}, + } + + # Before: todo is masked off despite not being in platform_toolsets yet. + assert "todo" not in _get_platform_tools(config, "cli") + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"file", "terminal", "todo"}) + + # After: todo must resolve as enabled, and untouched 'memory' must + # remain masked off. + resolved = _get_platform_tools(config, "cli") + assert "todo" in resolved + assert "memory" not in resolved + + +def test_save_platform_tools_no_disabled_toolsets_is_noop(): + """When agent.disabled_toolsets is absent or empty, the reconcile step + must be a complete no-op (no KeyError, no spurious 'agent' key creation). + """ + config = {"platform_toolsets": {"cli": ["file", "terminal"]}} + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"file", "terminal", "todo"}) + + assert "todo" in config["platform_toolsets"]["cli"] + # No 'agent' key should be fabricated when none existed. + assert "agent" not in config + + +def test_save_platform_tools_disabling_a_toolset_does_not_touch_disabled_toolsets(): + """Turning a toolset OFF (not present in enabled_toolset_keys) must not + remove anything from agent.disabled_toolsets -- only toolsets the user + just explicitly enabled are reconciled. + """ + config = { + "platform_toolsets": {"cli": ["file", "terminal", "todo"]}, + "agent": {"disabled_toolsets": ["memory"]}, + } + + with patch("hermes_cli.tools_config.save_config"): + # User unchecks 'todo' -- it's no longer in enabled_toolset_keys. + _save_platform_tools(config, "cli", {"file", "terminal"}) + + assert "todo" not in config["platform_toolsets"]["cli"] + # disabled_toolsets is untouched by a disable action. + assert config["agent"]["disabled_toolsets"] == ["memory"]