fix(tools): normalize numeric entries and clear stale no_mcp in _save_platform_tools

YAML parses bare numeric toolset names (e.g. 12306:) as int, causing
TypeError in sorted() since the read path normalizes to str but the
save path did not.

The no_mcp sentinel was preserved in existing entries even when the
user re-enabled MCP servers, causing MCP to stay silently disabled.
This commit is contained in:
alt-glitch
2026-04-24 15:55:51 +05:30
committed by Teknium
parent 5401a0080d
commit 9d7b64b5dd
2 changed files with 60 additions and 0 deletions

View File

@@ -717,6 +717,7 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[
existing_toolsets = config.get("platform_toolsets", {}).get(platform, [])
if not isinstance(existing_toolsets, list):
existing_toolsets = []
existing_toolsets = [str(ts) for ts in existing_toolsets]
# Preserve any entries that are NOT configurable toolsets and NOT platform
# defaults (i.e. only MCP server names should be preserved)
@@ -724,6 +725,11 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[
entry for entry in existing_toolsets
if entry not in configurable_keys and entry not in platform_default_keys
}
# Opening `hermes tools` is the user's opt-in to reconfigure tools, so treat
# saving from the picker as consent to clear the "no_mcp" sentinel. The
# picker has no checkbox for no_mcp, so without this users who once set it
# by hand could never re-enable MCP servers through the UI.
preserved_entries.discard("no_mcp")
# Merge preserved entries with new enabled toolsets
config["platform_toolsets"][platform] = sorted(enabled_toolset_keys | preserved_entries)

View File

@@ -601,3 +601,57 @@ class TestImagegenModelPicker:
_configure_imagegen_model("fal", config)
assert isinstance(config["image_gen"], dict)
assert config["image_gen"]["model"] == "fal-ai/flux-2/klein/9b"
def test_save_platform_tools_normalizes_numeric_entries():
"""YAML may parse bare numeric toolset names as int. They should be
normalized to str so they survive the save round-trip.
"""
config = {
"platform_toolsets": {
"cli": ["web", "terminal", 12306, "custom-mcp"]
}
}
with patch("hermes_cli.tools_config.save_config"):
_save_platform_tools(config, "cli", {"web", "browser"})
saved = config["platform_toolsets"]["cli"]
assert "12306" in saved
assert 12306 not in saved
def test_save_platform_tools_clears_no_mcp_sentinel():
"""`hermes tools` has no UI for no_mcp, so saving from the picker clears
the sentinel unconditionally — otherwise a user who once set no_mcp by
hand could never re-enable MCP servers through the UI.
"""
config = {
"platform_toolsets": {
"cli": ["web", "terminal", "no_mcp"]
}
}
with patch("hermes_cli.tools_config.save_config"):
_save_platform_tools(config, "cli", {"web", "browser"})
saved = config["platform_toolsets"]["cli"]
assert "no_mcp" not in saved
def test_save_platform_tools_preserves_mcp_server_names():
"""Non-sentinel passthrough entries (MCP server names) must still survive
the save — we only clear `no_mcp`, not every non-configurable entry.
"""
config = {
"platform_toolsets": {
"cli": ["web", "terminal", "custom-mcp", "another-mcp"]
}
}
with patch("hermes_cli.tools_config.save_config"):
_save_platform_tools(config, "cli", {"web", "browser"})
saved = config["platform_toolsets"]["cli"]
assert "custom-mcp" in saved
assert "another-mcp" in saved