fix(runtime-admin): swap dead agent.mcp_registry import for plugin helper (Phase 5) - #31
Merged
Merged
Conversation
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
2 |
First entries
plugins/myah-hermes-plugin/tests/test_runtime_admin_mcp_disconnect.py:32: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
plugins/myah-hermes-plugin/tests/test_runtime_admin_mcp_disconnect.py:33: [unresolved-import] unresolved-import: Cannot resolve imported module `aiohttp.test_utils`
✅ Fixed issues: none
Unchanged: 4193 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
…lper
The runtime-admin POST /myah/v1/admin/mcp/disconnect/{name} handler
imported disconnect_mcp_server from agent.mcp_registry — a module that
does not exist anywhere in the hermes-fork (or upstream). Every request
hit the 'except Exception' fallback and returned 500 with 'MCP registry
module not available', so the dashboard's DELETE /mcp/<name> chain
silently failed to actually disconnect MCP servers from
tools.mcp_tool._servers on the gateway side. The dashboard layer
captured the 500 and falls through to its own config-yaml mutation, so
the user saw 'success' while the gateway kept the stale reference until
the next process restart.
The correct module is myah_hermes_plugin.runtime_extensions.mcp_disconnect
(shipped in PR NousResearch#106, Phase E). It uses upstream's tools.mcp_tool
private state plus the _run_on_mcp_loop cross-loop bridge — exactly the
same pattern upstream's 'shutdown ALL servers' helper uses.
Tests in test_runtime_admin_mcp_disconnect.py drive the real handler
against a mocked tools.mcp_tool and assert the endpoint returns 200 +
ok=True (not the import-error 500 fallback). Pre-fix both tests fail
with ModuleNotFoundError: 'agent.mcp_registry'; post-fix both pass and
the fake server is actually popped from tools.mcp_tool._servers.
Related: docs/superpowers/plans/2026-05-11-no-fork-vendoring-respec.md (Phase 5)
deestax
force-pushed
the
fix/runtime-admin-mcp-disconnect-import
branch
from
May 11, 2026 11:52
9d53886 to
8c46217
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The runtime-admin
POST /myah/v1/admin/mcp/disconnect/{name}handler importeddisconnect_mcp_serverfromagent.mcp_registry— a module that does not exist anywhere in the repo (or upstream). Every request hit theexcept Exceptionfallback and returned 500 "MCP registry module not available".The dashboard's
DELETE /mcp/<name>route (inmyah_admin.dashboard._skills_plugins_mcp) delegates to this endpoint over HTTP. The dashboard captured the 500 and fell through to its ownconfig.yamlmutation, so users saw "disconnect succeeded" while the gateway kept the stale reference intools.mcp_tool._serversuntil process restart. Toolset cache + MCP-aware tool routing kept treating the server as connected until then.The fix
Swap the import to the real plugin-side helper at
myah_hermes_plugin.runtime_extensions.mcp_disconnect.disconnect_mcp_server. That module has shipped since PR NousResearch#106 (Phase E) — it uses upstream'stools.mcp_toolprivate state + the_run_on_mcp_loopcross-loop bridge, exactly the pattern upstream's "shutdown ALL servers" helper uses.One-line swap + a longer comment block explaining the resolution chain so future maintainers don't undo it.
Regression test
tests/test_runtime_admin_mcp_disconnect.pydrives the real handler from_make_handlers()against a mockedtools.mcp_toolstate (servers dict + threading lock +_run_on_mcp_loop) and asserts:{ok: True, name: "..."}(NOT the 500 import-error fallback)tools.mcp_tool._servers(cross-checks the chain end-to-end)ok=True(matches dashboard'sevict-allchain pattern)Pre-fix: both tests fail with
ModuleNotFoundError: No module named 'agent.mcp_registry'(TDD).Post-fix: both tests pass.
The existing
test_mcp_disconnect.pycovers the plugin helper in isolation; this new file covers the runtime-admin handler that calls into it — closes the integration gap.Companion PR ordering
Parent-repo submodule bump PR follows on
T3-Venture-Labs-Limited/myah. Merge this first.Refs
docs/superpowers/plans/2026-05-11-no-fork-vendoring-respec.md(Task 2: Phase 5 fix, ~0.5h estimate).