fix(profile/mcp): discover MCP tools after per-session HERMES_HOME mutation (#1968)

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Hermes Web UI -- Changelog
 
+## [Unreleased]
+
+### Fixed
+
+- **bug(profile/mcp): non-default profile MCP servers never load** ([#1968](https://github.com/nesquena/hermes-webui/issues/1968)). `_run_agent_streaming` called `discover_mcp_tools()` ~100 lines BEFORE the per-session `os.environ['HERMES_HOME'] = _profile_home` mutation, so MCP discovery always read the default profile's `~/.hermes/config.yaml` regardless of which profile the session was stamped with. Result: switching profiles in the WebUI dropdown was effectively cosmetic for MCP — non-default profiles never registered their stdio (npx/node) MCP servers. Fix relocates the `discover_mcp_tools()` call past the `_ENV_LOCK` env-mutation block so `get_hermes_home()` resolves to the session's actual profile home. Adds 4 static regression tests (`tests/test_issue1968_mcp_profile_discovery.py`) pinning the call ordering, lock-release placement, single call site, and try/except wrapping. **Caveat (out of scope, agent-side):** `_servers` in `tools/mcp_tool.py` is a process-global dict keyed only by server name, so concurrent use of multiple non-default profiles in the same WebUI process still has a "first profile wins per name" issue. Fully fixing that requires keying `_servers` by `(profile_home, name)` upstream in hermes-agent. This PR ships layer 1 only.
+
 ## [v0.51.31] — 2026-05-09 — Release H (12-PR contributor batch: image-mode + race fixes + composer drafts + locale parity)
 
 ### Added

api/streaming.py

@@ -1919,15 +1919,10 @@ def _run_agent_streaming(
     old_hermes_home = None
     old_profile_env = {}
 
-    # ── MCP Server Discovery (lazy import, idempotent) ──
-    # discover_mcp_tools() is called here (rather than at server startup) so that
-    # the hermes-agent package is fully initialized before we try to connect.
-    # It is safe to call multiple times — already-connected servers are skipped.
-    try:
-        from tools.mcp_tool import discover_mcp_tools
-        discover_mcp_tools()
-    except Exception:
-        pass  # MCP not available or not configured — non-fatal
+    # MCP discovery moved to AFTER the per-profile HERMES_HOME mutation below
+    # (was here at v0.51.30) — the previous placement always read the default
+    # profile's mcp_servers because os.environ['HERMES_HOME'] hadn't been
+    # rewritten yet.  See https://github.com/nesquena/hermes-webui/issues/1968.
 
     # Sprint 10: create a cancel event for this stream
     cancel_event = threading.Event()
@@ -2052,6 +2047,27 @@ def _agent_status_callback(kind, message):
             if _profile_home:
                 os.environ['HERMES_HOME'] = _profile_home
         # Lock released — agent runs without holding it
+        # ── MCP Server Discovery (lazy import, idempotent) ──
+        # MUST run AFTER the HERMES_HOME mutation above — `discover_mcp_tools()`
+        # reads `~/.hermes/config.yaml` via `get_hermes_home()`, which uses
+        # `os.environ['HERMES_HOME']`.  Calling it before the mutation always
+        # loaded the default profile's `mcp_servers`, even when the session
+        # was stamped with a non-default profile.  See issue #1968.
+        #
+        # NOTE: `_servers` in `tools/mcp_tool.py` is a process-global registry
+        # keyed by server name.  This means once profile A registers a server
+        # named e.g. `postgres`, profile B's discovery sees it as already
+        # connected and skips it — even if B's config points at a different
+        # binary.  Fully fixing multi-profile concurrent use requires keying
+        # `_servers` by `(profile_home, name)` upstream in hermes-agent; that
+        # lives outside this WebUI repo.  This change fixes the headline bug
+        # for users who run a single non-default profile per WebUI process.
+        try:
+            from tools.mcp_tool import discover_mcp_tools
+            discover_mcp_tools()
+        except Exception:
+            pass  # MCP not available or not configured — non-fatal
+
         # Register a gateway-style notify callback so the approval system can
         # push the `approval` SSE event the moment a dangerous command is
         # detected, without waiting for the next on_tool() poll cycle.

tests/test_issue1968_mcp_profile_discovery.py

@@ -0,0 +1,107 @@
+"""Regression test for issue #1968 — non-default profile MCP servers never load.
+
+The bug: `discover_mcp_tools()` was called at the top of `_run_agent_streaming`
+before the `HERMES_HOME` env mutation that stamps the per-session profile.
+Result: `_load_mcp_config()` always read the default profile's
+`~/.hermes/config.yaml`, never the non-default profile's MCP servers.
+
+The fix moves the call past the `_ENV_LOCK` env-mutation block so
+`discover_mcp_tools()` runs with the correct `HERMES_HOME` for the session's
+profile.
+
+This is a static check (source ordering) rather than a runtime test, because
+mocking the entire agent stack to reach the call site would be brittle and
+miss the actual lexical ordering that's the load-bearing fix.
+"""
+from pathlib import Path
+import re
+
+ROOT = Path(__file__).resolve().parents[1]
+STREAMING_PY = (ROOT / "api" / "streaming.py").read_text(encoding="utf-8")
+
+
+def _line_of(pattern: str) -> int:
+    """Return the 1-indexed line number of the first match for `pattern`."""
+    for idx, line in enumerate(STREAMING_PY.splitlines(), start=1):
+        if re.search(pattern, line):
+            return idx
+    raise AssertionError(f"pattern not found in api/streaming.py: {pattern!r}")
+
+
+def test_discover_mcp_tools_called_after_hermes_home_mutation():
+    """The fix for #1968: `discover_mcp_tools()` must execute AFTER the
+    `HERMES_HOME = _profile_home` assignment, otherwise non-default profile
+    MCP servers are never discovered.
+    """
+    home_set_line = _line_of(r"os\.environ\['HERMES_HOME'\]\s*=\s*_profile_home")
+    discover_call_line = _line_of(r"discover_mcp_tools\(\)\s*$")
+    assert discover_call_line > home_set_line, (
+        f"discover_mcp_tools() at line {discover_call_line} must be AFTER the "
+        f"HERMES_HOME mutation at line {home_set_line} (issue #1968). "
+        "Otherwise non-default profile MCP servers never load."
+    )
+
+
+def test_discover_mcp_tools_called_after_env_lock_release():
+    """`discover_mcp_tools()` should run AFTER the `_ENV_LOCK` block releases —
+    discovery itself can take up to 120s (per `_run_on_mcp_loop` timeout in
+    hermes-agent), and holding the env lock across that would serialize all
+    concurrent sessions through MCP discovery.
+
+    Lexical check: the discover call must come after the `# Lock released` marker
+    that follows the `with _ENV_LOCK:` block.
+    """
+    lock_release_marker = _line_of(r"# Lock released — agent runs without holding it")
+    discover_call_line = _line_of(r"discover_mcp_tools\(\)\s*$")
+    assert discover_call_line > lock_release_marker, (
+        f"discover_mcp_tools() at line {discover_call_line} should run AFTER "
+        f"the _ENV_LOCK release at line {lock_release_marker}, not inside the "
+        "lock block (which would serialize MCP discovery across sessions)."
+    )
+
+
+def test_discover_mcp_tools_only_called_once_in_streaming():
+    """Sanity check: only one *actual call* to `discover_mcp_tools()` in
+    `api/streaming.py` — not counting prose mentions inside comments.
+
+    The fix relocates the existing call rather than adding a second one.  If a
+    later refactor reintroduces a pre-mutation call site, this test catches it.
+    """
+    call_lines = [
+        line for line in STREAMING_PY.splitlines()
+        if "discover_mcp_tools()" in line
+        and not line.lstrip().startswith("#")
+    ]
+    assert len(call_lines) == 1, (
+        f"Expected exactly 1 `discover_mcp_tools()` call line in api/streaming.py "
+        f"(comments excluded), found {len(call_lines)}: {call_lines!r}.  A "
+        "duplicate call site would re-introduce the #1968 bug if placed before "
+        "the HERMES_HOME mutation."
+    )
+
+
+def test_discover_mcp_tools_call_is_inside_try_except():
+    """MCP discovery is best-effort — failures must not crash the chat stream.
+    Verify the call site is wrapped in `try: ... except Exception: pass`.
+
+    Looks at the 6 lines immediately surrounding the call (which is the actual
+    structural block, regardless of how chatty the preceding comment is).
+    """
+    lines = STREAMING_PY.splitlines()
+    call_idx = None
+    for idx, line in enumerate(lines):
+        if "discover_mcp_tools()" in line and not line.lstrip().startswith("#"):
+            call_idx = idx
+            break
+    assert call_idx is not None, "discover_mcp_tools() call line not found"
+    # Look at the 4 lines before and 4 lines after the call.
+    block_start = max(0, call_idx - 4)
+    block_end = min(len(lines), call_idx + 5)
+    block = "\n".join(lines[block_start:block_end])
+    assert "try:" in block, (
+        f"discover_mcp_tools() at line {call_idx + 1} must be inside a try block "
+        "so MCP failures don't crash the chat stream.  Surrounding code:\n" + block
+    )
+    assert "except" in block, (
+        f"discover_mcp_tools() at line {call_idx + 1} must have an except clause."
+    )