mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
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.<platform>, 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
This commit is contained in:
parent
020966574d
commit
3e16176ba4
2 changed files with 106 additions and 0 deletions
|
|
@ -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.<platform>,
|
||||
# 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)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue