Issue Station
Sign In
Tools hereToolbox

hermes - ✅(Solved) Fix [Bug]: _save_platform_tools preserves stale `no_mcp` and still crashes on numeric entries [1 pull requests]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…

Error Message

source venv/bin/activate && python - <<'PY' from hermes_cli.tools_config import _save_platform_tools, _get_platform_tools from unittest.mock import patch

config = { 'platform_toolsets': {'cli': ['web', 'no_mcp']}, 'mcp_servers': {'exa': {'url': 'https://mcp.exa.ai/mcp'}}, } with patch('hermes_cli.tools_config.save_config'): _save_platform_tools(config, 'cli', {'web', 'exa'}) print('saved:', config['platform_toolsets']['cli']) print('enabled after save:', sorted(_get_platform_tools(config, 'cli')))

config = {'platform_toolsets': {'cli': ['web', 12306]}} try: with patch('hermes_cli.tools_config.save_config'): _save_platform_tools(config, 'cli', {'web'}) except Exception as e: print(type(e).name, str(e)) PY

Fix Action

Fix / Workaround

Minimal Reproduction

source venv/bin/activate && python - <<'PY'
from hermes_cli.tools_config import _save_platform_tools, _get_platform_tools
from unittest.mock import patch

config = {
    'platform_toolsets': {'cli': ['web', 'no_mcp']},
    'mcp_servers': {'exa': {'url': 'https://mcp.exa.ai/mcp'}},
}
with patch('hermes_cli.tools_config.save_config'):
    _save_platform_tools(config, 'cli', {'web', 'exa'})
print('saved:', config['platform_toolsets']['cli'])
print('enabled after save:', sorted(_get_platform_tools(config, 'cli')))

config = {'platform_toolsets': {'cli': ['web', 12306]}}
try:
    with patch('hermes_cli.tools_config.save_config'):
        _save_platform_tools(config, 'cli', {'web'})
except Exception as e:
    print(type(e).__name__, str(e))
PY

PR fix notes

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

Description (problem / solution / changelog)

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

Changed files

  • hermes_cli/tools_config.py (modified, +16/-6)
  • tests/hermes_cli/test_tools_config.py (modified, +32/-0)

Code Example

source venv/bin/activate && python - <<'PY'
from hermes_cli.tools_config import _save_platform_tools, _get_platform_tools
from unittest.mock import patch

config = {
    'platform_toolsets': {'cli': ['web', 'no_mcp']},
    'mcp_servers': {'exa': {'url': 'https://mcp.exa.ai/mcp'}},
}
with patch('hermes_cli.tools_config.save_config'):
    _save_platform_tools(config, 'cli', {'web', 'exa'})
print('saved:', config['platform_toolsets']['cli'])
print('enabled after save:', sorted(_get_platform_tools(config, 'cli')))

config = {'platform_toolsets': {'cli': ['web', 12306]}}
try:
    with patch('hermes_cli.tools_config.save_config'):
        _save_platform_tools(config, 'cli', {'web'})
except Exception as e:
    print(type(e).__name__, str(e))
PY

---

saved: ['exa', 'no_mcp', 'web']
enabled after save: ['web']
TypeError: '<' not supported between instances of 'str' and 'int'
RAW_BUFFERClick to expand / collapse

Bug Description

_save_platform_tools() has two save-path bugs:

  1. it preserves the no_mcp sentinel even when the new selection explicitly re-enables MCP servers
  2. it can still crash on numeric toolset / MCP names during sorted(...)

Affected code:

  • hermes_cli/tools_config.py:566-597
  • related read path: hermes_cli/tools_config.py:540-559 / 481-483

Minimal Reproduction

source venv/bin/activate && python - <<'PY'
from hermes_cli.tools_config import _save_platform_tools, _get_platform_tools
from unittest.mock import patch

config = {
    'platform_toolsets': {'cli': ['web', 'no_mcp']},
    'mcp_servers': {'exa': {'url': 'https://mcp.exa.ai/mcp'}},
}
with patch('hermes_cli.tools_config.save_config'):
    _save_platform_tools(config, 'cli', {'web', 'exa'})
print('saved:', config['platform_toolsets']['cli'])
print('enabled after save:', sorted(_get_platform_tools(config, 'cli')))

config = {'platform_toolsets': {'cli': ['web', 12306]}}
try:
    with patch('hermes_cli.tools_config.save_config'):
        _save_platform_tools(config, 'cli', {'web'})
except Exception as e:
    print(type(e).__name__, str(e))
PY

Observed:

saved: ['exa', 'no_mcp', 'web']
enabled after save: ['web']
TypeError: '<' not supported between instances of 'str' and 'int'

Expected Behavior

  • Re-enabling an MCP server should clear any stale no_mcp disable-all sentinel.
  • Numeric YAML names should be normalized before sorting, just like the read path already normalizes them to strings.

Actual Behavior

Users can get stuck with MCP disabled, and some configs still crash on save.

Suggested Investigation Direction

Normalize existing_toolsets to strings up front and treat no_mcp as a special control flag instead of a preserved passthrough entry.

extent analysis

TL;DR

Normalizing existing_toolsets to strings and treating no_mcp as a special control flag can resolve the save-path bugs in _save_platform_tools().

Guidance

  • Normalize existing_toolsets to strings at the beginning of _save_platform_tools() to prevent crashes on numeric toolset/MCP names.
  • Treat no_mcp as a special control flag and remove it when an MCP server is re-enabled to prevent stale no_mcp disable-all sentinels.
  • Verify the fix by running the provided Minimal Reproduction code and checking that the saved output no longer includes no_mcp when an MCP server is re-enabled.
  • Review the related read path code (hermes_cli/tools_config.py:540-559 / 481-483) to ensure consistency in handling no_mcp and numeric toolset/MCP names.

Example

def _save_platform_tools(config, platform, toolsets):
    existing_toolsets = [str(tool) for tool in config['platform_toolsets'][platform]]
    # ...
    if 'no_mcp' in existing_toolsets and any(tool in toolsets for tool in config['mcp_servers']):
        existing_toolsets.remove('no_mcp')
    # ...

Notes

This fix assumes that the no_mcp sentinel is only used to disable all MCP servers and can be safely removed when an MCP server is re-enabled. Additional testing may be necessary to ensure that this fix does not introduce any regressions.

Recommendation

Apply the workaround by normalizing existing_toolsets to strings and treating no_mcp as a special control flag, as this directly addresses the identified issues and prevents crashes on numeric toolset/MCP names.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…
#environment setup#docker error#permission error#memory optimization#batch processing

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING