mcp: scope the server registry per profile — two profiles naming the same server share one subprocess and its credentials

[Izzy-Gottz]

The problem

The multiplexing gateway scopes config, memory and secrets per profile: _profile_runtime_scope installs a HERMES_HOME override alongside set_secret_scope, and agent/secret_scope.py is deliberately fail-closed so an un-migrated call site raises rather than silently reading another profile's value.

The MCP layer never joined that scheme. Thirteen module-level structures in tools/mcp_tool.py are keyed by bare server name, so two profiles that each configure a server called github share one subprocess — started by whichever arrived first, holding that profile's credentials.

A test written against unmodified code shows it directly:

test_two_profiles_same_server_name_do_not_share_a_connection
>       assert leaked is not server_a
E       AssertionError: profile B was handed profile A's 'github' MCP connection

Two things make it worse than a shared connection:

• _lazy_server_configs caches a config whose ${TOKEN} placeholders are already expanded. _load_mcp_config interpolates through the active secret scope before caching, so the cached dict literally holds the first profile's credential and would spawn the second profile's first call with it.
• The trust maps leak too. Inheriting another profile's trust: full or its readOnlyHint exemptions silently un-gates the dangerous-call approval path.

The fix

_mcp_profile_key() partitions every one of those structures:

• not a multiplexer → "", one partition, today's behaviour;
• multiplexing + scope → hermes_home_key(get_hermes_home_override()), the same override _profile_runtime_scope installs;
• multiplexing, no scope → a dedicated partition that cannot alias a profile (registries can't raise the way get_secret does).

This mirrors tools/registry.py::check_fn_cache_scope and hermes_cli/plugins.py::_plugin_home_key rather than introducing a second notion of identity. Gating on is_multiplex_active() rather than on "an override is set" is load-bearing: plugins, discovery, the bot-mode probe and the desktop backend all set overrides in single-profile processes, and partitioning on those would strand a server behind a key nothing looks up.

_ProfileScopedDict/_ProfileScopedSet implement the full MutableMapping/MutableSet protocols, so the ~90 existing call sites and test fixtures are unchanged; partitions()/for_key() are the escape hatch for genuinely process-global work.

Lifecycle. MCPServerTask captures its key at construction, because the long-lived run() task must address its own partition. shutdown_mcp_servers captures on the caller's context — run_coroutine_threadsafe does not copy contextvars onto the MCP loop. _stop_mcp_loop(only_if_idle=True) deliberately asks all partitions: the loop is process-global, and a per-profile check would let one profile's probe tear it out from under another's live servers.

Also fixed: gateway/run.py::_execute_mcp_reload ran shutdown_mcp_servers process-wide from inside one profile's scope, so one tenant typing /reload-mcp tore down every other tenant's stdio children — and, contextvars not propagating through run_in_executor, then re-discovered the default profile's mcp_servers.

Single-profile behaviour is unchanged

Three ways: by construction (the key is "" in any non-multiplexed process, so there is exactly one partition and every default is preserved); by explicit test (test_single_profile_gateway_is_unpartitioned asserts one partition even under a HERMES_HOME override); and by baseline diff — the same suites on the base commit and on this branch:

suite	baseline	with fix
76 MCP test files	845 passed, 7 failed	857 passed, 7 failed (+12 new)
gateway + cli + hermes_cli	15571 passed, 20 failed, 172 skipped	15571 passed, 20 failed, 172 skipped

Same failing files, same counts, both sides; those failures are pre-existing and platform-specific (macOS).

Known siblings, deliberately not fixed here

Reported rather than swept in, so this stays reviewable:

1. MCP tools are registered without scope=, so they land in the process-global _tools rather than _scoped_tools. Execution is profile-correct after this change (handlers re-resolve by name), but advertised schemas, names and descriptions still cross profiles. _scoped_tools already does this for plugins — the natural follow-up.
2. _mcp_stderr_log_fh resolves once under whichever profile calls first, then becomes every profile's stdio child's stderr.
3. hermes_cli/mcp_startup.py's _mcp_discovery_started is a once-per-process gate, so profiles 2..N get no MCP tools. Same shape in tui_gateway/entry.py.
4. _mcp_tool_server_names is left unpartitioned on purpose — it is meaningless apart from the global registry, and partitioning it alone would make has_registered_mcp_tools() lie. Fix with (1).
5. _MCP_DISCOVERY_LOCK_PATH; 6. mcp_oauth.py::_reserved_sockets (a global FIFO cap of 8, so one profile's OAuth burst can evict another's parked callback socket); 7. model_tools.py::_last_resolved_tool_names.

Happy to split any of those out if you'd rather see them separately.

Notes for review

• The _profile_key on MCPServerTask costs a __slots__ entry. Deliberate — trusting contextvar inheritance into the long-lived run() task is the kind of thing that breaks silently later.
• _mcp_profile_key() runs on every registry access including per-tool check_fn evaluation, so the two lazy imports are memoized at module level — the module is cached, not the functions, so monkeypatching is_multiplex_active still takes effect.
• Container annotations are generic over the value type so the advisory ty diff stays quiet; the blocking ruff rules are untouched.

[andrexibiza]

Reviewed exact head 2b871d4dd3cc34b30a9fad52f661fa68f9e48086 against base 59c5bd091332c51673d01b6639d2a7978a91e5d4, including the full 3-file diff, the 12 new isolation tests, current registry implementation, exact-head Actions, and the live MCP/profile PR/issue graph. The core diagnosis is correct, and there is good work here: partitioning the live connection, lazy interpolated config, trust/read-only metadata, breaker/backoff state, and carrying an explicit owner into long-lived MCPServerTask lifecycle closes a real cross-profile credential/authority leak. The profile_only loop-idleness check is also the right shape: the profile is local ownership, while the MCP loop is process ownership.

I do not think this exact head is safe to land yet, for three concrete reasons.

P1 — the mutation/runtime half is scoped, but the model-visible registry half is still process-global

This is acknowledged in the PR body, but it is not a harmless follow-up for this defect class. tools/registry.py::ToolRegistry.register(..., scope=None) writes into process-global _tools; _merged_tools() then includes _tools for every profile. The MCP registration paths on this head still call registry.register(...) without scope=, so a dynamic MCP tool discovered under profile A remains visible to profile B in schemas/names/descriptions and can replace the same globally named entry registered by B.

The important case is not merely metadata cosmetics. Let profiles A and B both name a logical server github, but return different schemas for the same raw tool name (or disjoint tool inventories). The process-global ToolEntry is whichever registration won last, while the handler on this branch re-resolves the server through the current profile partition. That permits a split-brain operation: model-visible schema/description/handler closure provenance from one profile, execution transport from the other. At minimum that leaks capability metadata and advertises tools a profile does not own; for same-named tools with different contracts it can drive arguments against a different profile's server under the wrong schema.

Please close the boundary as one ownership contract: dynamic MCP registrations, deregistration/refresh, toolset lookup/aliases, definitions, and dispatch must use the same canonical profile owner as _servers. _mcp_tool_server_names then needs to move with that owner (or disappear as redundant state). Add a real registry regression with two profiles, same server name, and deliberately different tool inventories plus deliberately different schemas for one same-named tool; prove A → B → A definitions and dispatch resolve the matching profile every time. Also prove an unscoped multiplex lookup cannot select either profile.

There is substantial prior work here that should be consolidated rather than silently reimplemented. #80746 is the earlier/open same-defect-class implementation: it scopes runtime and central registry by canonical profile home, including schema/handler/status/dispatch, and it was updated on Aug 30 with a production reproduction of wrong-profile authenticated MCP transport. This PR has useful newer mechanics and a narrower container approach; #80746 has unique registry-side coverage. Treat them as overlapping implementations with salvage/credit on both sides, not as unrelated siblings or as a reason to discard either contributor's work.

P1 — cold-start discovery is still on the other side of the boundary

This head fixes profile-routed /reload-mcp, but a fresh multiplexed gateway still needs a path that populates each profile's partition. #95518 documents the current failure: gateway boot discovery reads the default home, the turn path only snapshots the registry, and a profile-only mcp_servers config can remain undiscovered until some unrelated scoped path happens to run. #95542 is the current complementary implementation for exactly that surface: per-profile gateway boot discovery, executor-context propagation, profile-local registration/shutdown, and a gateway-level regression.

The new tests/tools/test_mcp_profile_isolation.py is strong for container ownership, but it constructs fake servers directly and does not execute gateway boot discovery or _execute_mcp_reload. Since this PR changes gateway/run.py, please add/compose a gateway integration test that starts from empty MCP state, supplies two served profile homes, and proves both profile configs are discovered without a cron/reload side effect; then prove /reload-mcp for B cannot tear down A and cannot repopulate from the default home. The cleanest route may be to consolidate the distinct pieces of #95542 rather than create a third implementation of its gateway half. Preserve #95542's contributor credit for that path.

#96147 is adjacent rather than duplicate: it adds authenticated-requester identity below the profile boundary for OAuth and has already had to solve scoped reload/cache teardown cases. Its requester dimension should remain a refinement of (profile owner, server) rather than being flattened by whichever profile solution lands.

Merge-order / ownership blocker — this head writes through an active mcp_tool.py sharding owner

The FILE-LIST is not clean. #84070 is an open, recently updated byte-verbatim extraction whose exact ownership window includes _server_connect_retry_after and _server_connect_failures, moving those names into tools/mcp_connect_cooldown.py. This PR changes those exact state owners in-place and adds a large new profile-container implementation to tools/mcp_tool.py, which is already far beyond the repository's 2K decomposition boundary.

Please resolve that land order rather than making one branch invalidate the other's seam contract. If #84070 lands first, the profile-keyed cooldown state belongs in the extracted bounded owner (or in a dedicated profile-state module) and this branch should consume/re-export it. If this work lands first, #84070 needs an explicit restack/new golden against the new state shape while preserving its authorship and extraction provenance. More broadly, the profile partition container itself is coherent enough to be a bounded owner; it should not become hundreds of additional lines of permanent godfile surface.

Acceptance state

The PR reports useful local before/after suites, but there is no hosted exact-head acceptance receipt yet. At 2b871d4dd3cc34b30a9fad52f661fa68f9e48086, CI, Docker, and Nix are all currently action_required (not green runs). That is not evidence of a code failure, but it does mean this commit is not acceptance-green yet.

I would keep the partition/lifecycle work — especially the explicit MCPServerTask._profile_key, lazy-config isolation, trust-map isolation, and process-global idle check. Those are real fixes. The landing unit just needs to finish the same identity all the way through discovery → registry/schema → live connection → dispatch → teardown, reconcile the existing owners (#80746, #95542, #96147, #84070), and then get exact-head hosted green. That will turn this from a strong partial isolation fix into one coherent profile-ownership boundary.

[Izzy-Gottz]

Thank you — this is a more careful read than the PR deserved, and the split-brain argument lands.

On P1 (registry). You are right and I was wrong to file it as a follow-up. I reasoned "execution is profile-correct, so the rest is metadata," and that stops being true exactly where you point: with the handler re-resolving the server per partition but the ToolEntry process-global and last-writer-wins, model-visible schema and execution transport can come from different profiles. For two profiles whose same-named servers expose different contracts, that is arguments driven against the wrong server under the wrong schema — a correctness bug, not cosmetics. _mcp_tool_server_names was left unpartitioned on the reasoning that it is meaningless apart from the global registry; under one ownership contract it should move with the owner or disappear, and I agree with you that it is the latter.

On overlap. I did not know about #80746, #95542 or #96147 when I wrote this, and having read them now, I do not think a fourth implementation helps anyone. #80746 covers the registry side I left open and has a production reproduction; #95542 covers gateway boot discovery, which my tests deliberately avoid because they construct fake servers rather than exercising discovery. I would rather this be salvaged into those than land beside them. Concretely, what looks uniquely useful here is the explicit MCPServerTask._profile_key (carrying the owner into the long-lived run() task rather than trusting contextvar inheritance), the lazy-config isolation — the cached config holds credentials already interpolated through the active secret scope, which I think is the sharpest edge in this defect class — the trust/read-only maps, and the process-global idle check on _stop_mcp_loop. Happy for those to arrive as commits on #80746 with credit as you see fit, or to reduce this PR to just those pieces on top of it. Your call which is less work to review.

On #84070. Agreed that this head writes through its extraction window, and I would rather not invalidate its seam. My preference is that #84070 lands first and the profile-keyed cooldown state goes into the extracted owner rather than staying in mcp_tool.py — that also addresses your godfile point, since the partition container is coherent enough to be its own bounded owner rather than several hundred more lines in that file. If you would rather this land first, say so and I will restack.

On acceptance. Understood that local before/after is not a hosted receipt. There are no checks reported on the head at all, which I take to be the first-time-contributor approval gate rather than a result — if you approve a run I will chase whatever it surfaces.

So: happy to do the discovery → registry/schema → connection → dispatch → teardown work as one boundary with a real gateway integration test, but only in whichever shape avoids duplicating #80746 and #95542. Tell me the landing order you want and I will follow it.

[teknium1]

Heads-up, @Izzy-Gottz: the gateway half of this is on main. ee0e234 runs MCP discovery once per served profile inside _profile_runtime_scope + copy_context() and gives tools/mcp_tool.py a scope layer (_server_scope_keys, _server_tool_scopes, _server_visible_in_scope) so a multiplexed /reload-mcp tears down only its own profile's servers and each profile sees only its adopted connections.

What main did not take from this PR — and why it stays open — is the registry partition itself: _servers and the connect-error / cooldown / trust / breaker state are still keyed by bare server name, so two profiles configuring the same name still share one connection object (the split-brain case your P1 discussion lands on). A rebase onto the overlay model that partitions those maps by registry.current_scope_key() rather than introducing a parallel scope key would be the reviewable shape.

[teknium1]

Superseded by #108352 (on main as ceaf622), with Co-authored-by credit to you as the earliest fix. You proposed a per-profile registry; what shipped keeps one registry and keys every ledger by (owner_scope, name) via the new tools/mcp_tool_scope.py, with credential-aware adoption of an identical connection and re-registration of adopters after an owner /reload-mcp. Same outcome you were after — two profiles naming one server no longer share a subprocess or its credentials. Thanks, @Izzy-Gottz.