Skip to content

fix(cli): clear stale no_mcp and normalize numeric toolset names on save (#13028) - #13086

Closed
dingn42 wants to merge 1 commit into
NousResearch:mainfrom
dingn42:fix/save-platform-tools-stale-no-mcp
Closed

fix(cli): clear stale no_mcp and normalize numeric toolset names on save (#13028)#13086
dingn42 wants to merge 1 commit into
NousResearch:mainfrom
dingn42:fix/save-platform-tools-stale-no-mcp

Conversation

@dingn42

@dingn42 dingn42 commented Apr 20, 2026

Copy link
Copy Markdown

Problem

_save_platform_tools() has two separate save-path defects (both reproducible by running the Python snippets in the issue body):

  1. When a platform previously had the no_mcp disable-all sentinel and the user re-enables an MCP server, the sentinel survives the preserve-filter and silently re-suppresses the newly selected server on the next read. Observed: _get_platform_tools returned ['web'] immediately after saving {'web', 'exa'}.
  2. YAML may parse bare numeric toolset names (e.g. 12306:) as int. The read path in _get_platform_tools already normalizes to str (lines 522-524), but the save path sorted mixed str/int entries and crashed with TypeError: '<' not supported between instances of 'str' and 'int'.

Fixes #13028.

Fix

  • Normalize existing_toolsets to strings up front so sorted() on the merged set stays monomorphic — mirrors what _get_platform_tools already does on the read side.
  • When enabled_toolset_keys contains any MCP server name (resolved from config['mcp_servers']), drop the no_mcp sentinel from preserved_entries. Plain saves that don't touch MCP still keep the sentinel.

Test plan

  • Added two regression tests in tests/hermes_cli/test_tools_config.py:
    • test_save_platform_tools_clears_stale_no_mcp_when_enabling_mcp_server
    • test_save_platform_tools_normalises_numeric_existing_entries
  • pytest tests/hermes_cli/test_tools_config.py — 34 passed
  • Reran both Python reproducers from the issue against the fixed branch — first case: ['exa', 'web'] saved, ['exa', 'web'] enabled after reload; second case: no TypeError, 12306 round-trips as string

…ave (NousResearch#13028)

_save_platform_tools had two edge-case defects:

1. When a platform previously had the "no_mcp" disable-all sentinel and
   the user re-enabled an MCP server, the sentinel survived through the
   preserve-filter and silently re-suppressed the newly selected server
   on the next read.

2. YAML may parse bare numeric toolset names (e.g. ``12306:``) as int.
   The read path in _get_platform_tools already normalizes to str, but
   the save path sorted mixed str/int entries and crashed with
   ``TypeError: '<' not supported between instances of 'str' and 'int'``.

Normalize existing_toolsets to strings up front and drop the stale
no_mcp entry whenever the new selection re-enables any MCP server.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the thorough write-up and repro steps, @dingn42! Both fixes landed on main in a parallel commit before this PR could be merged.

Automated hermes-sweeper review found the following evidence:

  • hermes_cli/tools_config.py line 802: existing_toolsets = [str(ts) for ts in existing_toolsets] — numeric normalization fix is present
  • hermes_cli/tools_config.py line 814: preserved_entries.discard("no_mcp") — stale sentinel fix is present
  • Implementing commit: 9d7b64b5dd394a60df7d38824256e2f213eac5c6 (fix(tools): normalize numeric entries and clear stale no_mcp in _save_platform_tools, merged 2026-04-24)
  • Regression tests for both cases added to tests/hermes_cli/test_tools_config.py (lines 606 and 624)

The main implementation uses unconditional no_mcp discard on any hermes tools save (slightly broader than the MCP-server-name-conditional approach in this PR), but the outcome is the same for all the cases described in #13028. Closing as implemented. Your diagnosis and tests were valuable — the parallel fix picked up the same approach!

@teknium1 teknium1 closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: _save_platform_tools preserves stale no_mcp and still crashes on numeric entries

3 participants