Skip to content

fix(mcp): isolate runtime ownership by profile - #80746

Open
franksong2702 wants to merge 3 commits into
NousResearch:mainfrom
franksong2702:franksong2702/mcp-profile-ownership-metadata
Open

fix(mcp): isolate runtime ownership by profile#80746
franksong2702 wants to merge 3 commits into
NousResearch:mainfrom
franksong2702:franksong2702/mcp-profile-ownership-metadata

Conversation

@franksong2702

@franksong2702 franksong2702 commented Aug 7, 2026

Copy link
Copy Markdown

What does this PR do?

Hermes' MCP runtime and central tool registry were process-global by server/tool name. In a multiplex process, two profiles configuring the same MCP server name could overwrite or observe each other's status, schema, handler, lazy state, and reconnect bookkeeping.

This PR makes canonical (profile_home, server_name) the runtime owner and scopes MCP registry entries by canonical profile home.

Changes made

  • Key connected, connecting, failed, retry, circuit-breaker, parallel-safety, and lazy MCP state by (profile_home, server_name).
  • Scope MCP registry schemas, handlers, toolset checks, aliases, lookups, dispatch, and teardown by canonical profile home.
  • Capture the owner in eager, lazy-cache, first-use connect, reconnect, and dynamic refresh paths.
  • Keep built-in/plugin registry entries process-global.
  • Fail closed for unscoped multiplex status, discovery, registry, reconnect, and provenance queries.
  • Preserve explicit owner provenance in every MCP status row and registry entry.
  • Add a two-profile duplicate-name regression proving A → B → A status, schema, and dispatch isolation.

This is the Agent-side contract consumed by Hermes WebUI #6114 at WebUI head c93c50874f57d5f13fa93ff34494d2dfe4021338.

Related issue

Type of change

  • Bug fix
  • Tests

Verification

Fail-before: the new profile-scoped registry regressions initially reported 4 failures because the API did not accept an owner dimension and an unscoped lookup could select another profile's entry.

Head verification:

./scripts/run_tests.sh tests/tools/test_registry.py
# 34 passed

./scripts/run_tests.sh tests/tools/test_mcp_tool.py
# 96 passed

./scripts/run_tests.sh tests/tools/test_mcp_*.py tests/tools/test_refresh_agent_mcp_tools.py tests/tools/test_registry.py
# 50 files, 471 passed, 0 failed

./.venv/bin/ruff check tools/registry.py tools/mcp_tool.py tests/tools/test_registry.py tests/tools/test_mcp_tool.py tests/tools/test_mcp_dynamic_discovery.py tests/tools/test_mcp_lazy_start.py
# All checks passed

python3 -m py_compile tools/registry.py tools/mcp_tool.py tests/tools/test_registry.py tests/tools/test_mcp_tool.py tests/tools/test_mcp_dynamic_discovery.py tests/tools/test_mcp_lazy_start.py
git diff --check upstream/main...HEAD
# both exit 0

Cross-repository consumer verification:

HERMES_WEBUI_AGENT_DIR=/path/to/this/worktree ./scripts/test.sh tests/test_issue5619_profile_config_isolation.py tests/test_issue697_mcp_tool_inventory.py
# 47 passed

The repository-wide test command was also attempted locally. Its non-green nodes are outside the changed MCP/registry files and include missing optional acp / anthropic SDKs; no green full-suite claim is made.

Scope

No config keys, dependencies, visual surfaces, deployment behavior, or WebUI code are changed here. Built-in and plugin tool behavior remains global.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth area/profiles Multi-profile isolation, HERMES_HOME scoping labels Aug 7, 2026
@franksong2702
franksong2702 force-pushed the franksong2702/mcp-profile-ownership-metadata branch from d0d0afc to 98b4e34 Compare August 7, 2026 10:50
@franksong2702 franksong2702 changed the title fix(mcp): expose profile ownership provenance fix(mcp): isolate runtime ownership by profile Aug 7, 2026
@franksong2702
franksong2702 force-pushed the franksong2702/mcp-profile-ownership-metadata branch from 98b4e34 to 7ecac2e Compare August 8, 2026 01:27
@franksong2702

Copy link
Copy Markdown
Author

Rebased this PR onto current upstream/main (b3aa561fa) and force-pushed exact head 7ecac2e7e with lease protection.

Post-rebase review found one remaining ownership leak introduced by the newer trust-tier gate: trust/read-only metadata was still keyed only by raw server name, so two profiles using the same MCP server name could overwrite each other's approval policy. The new head keys that metadata with the same (canonical profile home, server name) identity already used by runtime, registry, status, and dispatch.

Regression proof:

  • Before the fix, the new two-profile behavior test failed because profile A (trust: full) was incorrectly blocked by profile B's trust: untrusted metadata.
  • After the fix: 168/168 passed across test_registry.py, test_mcp_tool.py, test_mcp_dynamic_discovery.py, test_mcp_lazy_start.py, and neighboring test_mcp_trust_gating.py.
  • git diff --check passes.

The PR diff remains limited to the original six MCP runtime/registry test and implementation files.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have tool/mcp MCP client and OAuth type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants