Skip to content

fix(tools): diff /reload-mcp against config, not the connection registry - #80855

Open
bong-u wants to merge 1 commit into
NousResearch:mainfrom
bong-u:fix/reload-mcp-diff-reads-config
Open

bong-u wants to merge 1 commit into
NousResearch:mainfrom
bong-u:fix/reload-mcp-diff-reads-config

Conversation

@bong-u

@bong-u bong-u commented Aug 7, 2026

Copy link
Copy Markdown

What does this PR do?

/reload-mcp diffed _servers, the live connection registry, which only holds servers with an established session. Anything configured but not connected at that moment printed as ➖ Removed: still mid-connect, enabled: false, serving a post-failure backoff, or lazily registered from the schema cache.
The same wording went into conversation history, which tells the model a working server is gone.

Now only a name absent from config.yaml counts as a removal. Anything else still in config shows its real state:

♻️ Reconnected: github
⏳ Still configured, not connected: notion (connecting), linear (disabled)

cli.py:_reload_mcp and gateway/run.py:_execute_mcp_reload held identical copies of the diff, so both now call tools/mcp_tool.py:summarize_mcp_reload().

Availability comes from one get_mcp_status() snapshot rather than _servers membership, which is wrong in both directions.
A parked server sits in _servers with session=None, so a membership check calls it reconnected while /mcp shows it failed.
A lazily registered server has callable tools and no _servers entry at all, so a healthy lazy: true config would report it as not connected on every reload.

Related Issue

Fixes #80771

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/mcp_tool.py: summarize_mcp_reload(old_servers) returns connected / added / reconnected / removed / pending.
  • cli.py, gateway/run.py: both reload paths call it and print the new line in the output and in the history note.
    The helper blocks on _lock, so the gateway calls it through run_in_executor.
  • locales/*.yaml: gateway.reload_mcp.pending in all 17 catalogs (tests/agent/test_i18n.py asserts parity).
  • tests/tools/test_mcp_reload_diff.py: still configured, dropped from config, newly added, lazy.

How to Test

A deterministic version of the issue's timing-dependent repro:

  1. Start hermes with a working MCP server and confirm it with /mcp.
  2. Set enabled: false on that server in config.yaml.
  3. Run /reload-mcp.

Before: ➖ Removed: <server>.
After: ⏳ Still configured, not connected: <server> (disabled).

Also worth checking: an unreachable command reports (failed), and deleting the server from config.yaml still reports ➖ Removed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.6 (arm64), Python 3.11.15

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

$ scripts/run_tests.sh tests/tools/test_mcp_reload_diff.py tests/agent/test_i18n.py
=== Summary: 2 files, 40 tests passed, 0 failed (100% complete) in 11.1s (16 workers) ===

/reload-mcp diffed `_servers`, which holds only servers with a live
session. Anything configured but not connected at that instant came out
as "➖ Removed": a server still mid-connect, one gated `enabled: false`,
one serving a post-failure backoff, or one lazily registered from the
schema cache. The same wrong wording went into conversation history,
telling the model a working server was gone.

cli.py `_reload_mcp` and gateway/run.py `_execute_mcp_reload` carried
identical copies of the diff, so both now call a shared
`summarize_mcp_reload()`. Only a name absent from config.yaml counts as
a removal. Anything else still in config renders as "⏳ Still
configured, not connected: name (connecting/failed/disabled)".

Availability now comes from a single `get_mcp_status()` snapshot instead
of `_servers` membership, which is wrong in both directions: a parked
server sits in `_servers` with `session=None` and would be announced as
reconnected, while a lazily registered one has callable tools and no
`_servers` entry and would be listed as not connected.

Adds `gateway.reload_mcp.pending` to all 17 locale catalogs (test_i18n
asserts parity) and a unit test for the diff.

Fixes NousResearch#80771
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery tool/mcp MCP client and OAuth area/config Config system, migrations, profiles sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists labels Aug 7, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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]: /reload-mcp reports still-configured servers as "➖ Removed" — the diff never reads config

2 participants