mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-27 17:58:07 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(mcp): propagate profile HERMES_HOME override into shared discovery owner; restore stdio startup + WSTransport tests

Browse source 
Follow-up to @LionGateOS's #72135 salvage:
- Route ensure_mcp_discovery_started through hermes_cli.mcp_startup's
  shared owner instead of a hand-rolled bare thread, keeping the start
  lock, retry-after-zero-connected allowance, and interactive-OAuth
  suppression. The shared owner now captures the caller's context-local
  HERMES_HOME override and re-installs it inside the discovery thread,
  so discovery reads the selected profile's mcp_servers (#67605).
- Restore stdio TUI startup discovery in main() and the
  _mcp_discovery_enabled retry gate in wait_for_mcp_discovery, both
  dropped by the original branch.
- Restore the 3 WSTransport regression tests (send serialization,
  cross-batch ordering, drained-token ordering) deleted by the PR.
- Harden the profile-scoped discovery test against sibling-state leaks.
This commit is contained in:
teknium1 2026-07-26 14:02:33 -07:00 • committed by Teknium
parent a7bb123b4a
commit af8d698b41
4 changed files with 220 additions and 58 deletions
Download patch file Download diff file Expand all files Collapse all files

28
hermes_cli/mcp_startup.py
View file

@ -58,7 +58,28 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None:
if not _has_configured_mcp_servers():
return
# Capture the caller's context-local HERMES_HOME override (profile
# scoping in multi-profile processes like the dashboard/desktop
# backend) and re-install it inside the discovery thread. ContextVars
# do not propagate into bare threads, so without this a session
# "switched" to profile X would discover the LAUNCH profile's
# mcp_servers instead (#67605). The config gate above already runs on
# the caller's thread, so it sees the same override.
try:
from hermes_constants import get_hermes_home_override
home_override = get_hermes_home_override()
except Exception:
home_override = None
def _discover() -> None:
token = None
try:
from hermes_constants import set_hermes_home_override
token = set_hermes_home_override(home_override)
except Exception:
token = None
try:
_discover_mcp_tools_without_interactive_oauth()
try:
@ -73,6 +94,13 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None:
except Exception:
logger.debug("Background MCP tool discovery failed", exc_info=True)
finally:
if token is not None:
try:
from hermes_constants import reset_hermes_home_override
reset_hermes_home_override(token)
except Exception:
pass
with _mcp_discovery_lock:
global _mcp_discovery_thread, _mcp_discovery_started
_mcp_discovery_thread = None

15
tests/test_tui_gateway_server.py
View file

@ -569,6 +569,7 @@ def _write_profile_cfg(home: Path, cwd: str | None) -> Path:
def test_profile_scoped_mcp_discovery_uses_target_home(monkeypatch, tmp_path):
"""MCP discovery must start under the selected profile's HERMES_HOME."""
from hermes_cli import mcp_startup
from hermes_constants import get_hermes_home
from tui_gateway import entry
@ -587,20 +588,26 @@ def test_profile_scoped_mcp_discovery_uses_target_home(monkeypatch, tmp_path):
seen = []
monkeypatch.setattr(entry, "_mcp_discovery_thread", None)
monkeypatch.setattr(mcp_startup, "_mcp_discovery_started", False)
monkeypatch.setattr(mcp_startup, "_mcp_discovery_thread", None)
# ensure_mcp_discovery_started flips this module global; monkeypatch it so
# the enablement doesn't leak into sibling tests in this file.
monkeypatch.setattr(entry, "_mcp_discovery_enabled", False)
monkeypatch.setattr(
"tools.mcp_tool.discover_mcp_tools",
mcp_startup,
"_discover_mcp_tools_without_interactive_oauth",
lambda: seen.append(str(get_hermes_home())),
)
try:
entry.ensure_mcp_discovery_started()
thread = entry._mcp_discovery_thread
thread = mcp_startup._mcp_discovery_thread
assert thread is not None
thread.join(timeout=2)
finally:
reset_hermes_home_override(token)
entry._mcp_discovery_thread = None
mcp_startup._mcp_discovery_thread = None
mcp_startup._mcp_discovery_started = False
assert seen == [str(profile_home)]

118
tests/test_tui_gateway_ws.py
View file

@ -218,3 +218,121 @@ def test_ws_starts_mcp_discovery_before_ready(monkeypatch):
# should not start MCP discovery before a profile has been bound.
assert calls == []
assert events == ["accept", "ready_after_0"]
def test_ws_transport_serializes_concurrent_sends():
active_sends = 0
max_active_sends = 0
sent = []
class FakeWS:
async def send_text(self, line):
nonlocal active_sends, max_active_sends
active_sends += 1
max_active_sends = max(max_active_sends, active_sends)
try:
await asyncio.sleep(0.05)
sent.append(line)
finally:
active_sends -= 1
loop = asyncio.new_event_loop()
thread = threading.Thread(target=loop.run_forever, daemon=True)
thread.start()
try:
transport = ws_mod.WSTransport(FakeWS(), loop, peer="serialize-test")
with concurrent.futures.ThreadPoolExecutor(max_workers=2) as pool:
futures = [
pool.submit(transport.write, {"idx": 1}),
pool.submit(transport.write, {"idx": 2}),
]
assert [f.result(timeout=2) for f in futures] == [True, True]
assert len(sent) == 2
assert max_active_sends == 1
assert transport._closed is False
finally:
loop.call_soon_threadsafe(loop.stop)
thread.join(timeout=2)
loop.close()
def test_ws_transport_preserves_cross_batch_order():
async def scenario():
entered = []
first_entered = asyncio.Event()
release_first = asyncio.Event()
second_started = asyncio.Event()
class FakeWS:
async def send_text(self, line):
entered.append(line)
if line == "A1":
first_entered.set()
await release_first.wait()
transport = ws_mod.WSTransport(
FakeWS(), asyncio.get_running_loop(), peer="batch-order-test"
)
first = asyncio.create_task(transport._safe_send_many(["A1", "A2"]))
await first_entered.wait()
async def send_second():
second_started.set()
await transport._safe_send_many(["B1", "B2"])
second = asyncio.create_task(send_second())
await second_started.wait()
# The second task has reached the transport. Without whole-batch
# serialization it runs B1/B2 before this task can resume.
assert entered == ["A1"]
release_first.set()
await asyncio.gather(first, second)
assert entered == ["A1", "A2", "B1", "B2"]
asyncio.run(scenario())
def test_ws_write_async_keeps_drained_tokens_with_current_frame():
async def scenario():
entered = []
first_entered = asyncio.Event()
release_first = asyncio.Event()
current_started = asyncio.Event()
class FakeWS:
async def send_text(self, line):
entered.append(line)
if line == "A1":
first_entered.set()
await release_first.wait()
transport = ws_mod.WSTransport(
FakeWS(), asyncio.get_running_loop(), peer="async-order-test"
)
transport._pending_tokens.append("pending-token")
first = asyncio.create_task(transport._safe_send_many(["A1", "A2"]))
await first_entered.wait()
async def send_current():
current_started.set()
await transport.write_async({"id": "current"})
current = asyncio.create_task(send_current())
await current_started.wait()
later = asyncio.create_task(transport._safe_send_many(["later-batch"]))
release_first.set()
await asyncio.gather(first, current, later)
assert entered == [
"A1",
"A2",
"pending-token",
json.dumps({"id": "current"}),
"later-batch",
]
asyncio.run(scenario())

117
tui_gateway/entry.py
View file

@ -24,20 +24,23 @@ from tui_gateway.transport import TeeTransport
logger = logging.getLogger(__name__)
# Handle for the background MCP tool-discovery thread (see main()). The first
# agent build briefly joins this so already-spawning fast servers land before
# the agent snapshots its tool list (see wait_for_mcp_discovery).
# Handle for the background MCP tool-discovery thread (see
# ensure_mcp_discovery_started). The first agent build briefly joins this so
# already-spawning fast servers land before the agent snapshots its tool list
# (see wait_for_mcp_discovery). Stays None when discovery is delegated to the
# shared owner in hermes_cli.mcp_startup — the wait/in-flight/join helpers
# below consult both owners.
_mcp_discovery_thread = None
# True once main() decided this TUI process has MCP servers configured and
# spawned discovery through the shared owner. Lets wait_for_mcp_discovery
# re-invoke the (idempotent) spawn on later agent builds so the
# retry-after-zero-connected allowance in
# hermes_cli.mcp_startup.start_background_mcp_discovery can actually fire for
# the stdio TUI — without this, main()'s single spawn is the only call and a
# first run that connected nothing latches the process MCP-less. Kept as a
# flag (rather than re-probing config) so non-MCP sessions never pay the
# tools.mcp_tool import on the per-agent-build wait path.
# True once ensure_mcp_discovery_started decided this process has MCP servers
# configured and spawned discovery through the shared owner. Lets
# wait_for_mcp_discovery re-invoke the (idempotent) spawn on later agent
# builds so the retry-after-zero-connected allowance in
# hermes_cli.mcp_startup.start_background_mcp_discovery can actually fire —
# without this, the single spawn is the only call and a first run that
# connected nothing latches the process MCP-less. Kept as a flag (rather than
# re-probing config) so non-MCP sessions never pay the tools.mcp_tool import
# on the per-agent-build wait path.
_mcp_discovery_enabled = False
@ -243,17 +246,18 @@ def wait_for_mcp_discovery(timeout: "float | None" = None) -> None:
bound = timeout if timeout is not None else 0.75
thread.join(timeout=bound)
return
# The stdio TUI spawns discovery via the shared owner (see main()); wait
# on it so the first agent build still catches fast servers. Re-invoke
# the idempotent spawn first: if the previous run finished with zero
# connected servers, start_background_mcp_discovery's
# retry-after-zero-connected allowance kicks off a fresh discovery run
# here instead of leaving the TUI latched MCP-less for the session.
# Only the stdio TUI (which spawned discovery through the shared owner)
# should delegate to the startup wait here — for every other surface
# (dashboard /api/ws) _make_agent already calls
# hermes_cli.mcp_startup.wait_for_mcp_discovery directly, and delegating
# unconditionally would make that bounded wait run twice per agent build.
# Discovery is spawned via the shared owner (ensure_mcp_discovery_started
# → hermes_cli.mcp_startup); wait on it so the first agent build still
# catches fast servers. Re-invoke the idempotent spawn first: if the
# previous run finished with zero connected servers,
# start_background_mcp_discovery's retry-after-zero-connected allowance
# kicks off a fresh discovery run here instead of leaving the process
# latched MCP-less for the session. In multi-profile processes this
# retry runs under the CALLER's profile context (agent build binds the
# session profile's HERMES_HOME first), so a launch profile with no
# mcp_servers no longer starves selected profiles of discovery (#67605).
# Gated on _mcp_discovery_enabled so non-MCP sessions never pay the
# tools.mcp_tool import on the per-agent-build wait path.
if not _mcp_discovery_enabled:
return
try:
@ -356,48 +360,53 @@ def _has_configured_mcp_servers() -> bool:
def ensure_mcp_discovery_started() -> None:
"""Start background MCP discovery once for gateway entrypoints.
"""Start background MCP discovery for the current profile context, once.
``main()`` covers the stdio/TUI path. WebSocket/Desktop entrypoints can
accept sessions without running ``main()``, so they must call this helper
before the first agent snapshots its tool list.
``main()`` calls this for the stdio/TUI path. WebSocket/Desktop
entrypoints can accept sessions without running ``main()``, so the
agent-build path (``server._start_agent_build``) also calls it AFTER
binding the session profile's HERMES_HOME override — the shared owner in
``hermes_cli.mcp_startup`` captures the caller's context-local override
and propagates it into the discovery thread, so discovery reads the
SELECTED profile's ``mcp_servers``, not the launch profile's (#67605).
Delegating to the shared owner (instead of a hand-rolled thread) keeps
the process-wide start lock, the retry-after-zero-connected allowance,
and interactive-OAuth suppression.
Known limitation: MCP tool registration is process-global, so in a
multi-profile process the FIRST profile that builds an agent wins the
discovery slot. Full per-profile MCP registries are tracked in #67605.
"""
global _mcp_discovery_thread
from hermes_constants import get_hermes_home_override, reset_hermes_home_override, set_hermes_home_override
if _mcp_discovery_thread is not None:
return
global _mcp_discovery_enabled
if not _has_configured_mcp_servers():
return
_mcp_discovery_enabled = True
try:
from hermes_cli.mcp_startup import start_background_mcp_discovery
home_override = get_hermes_home_override()
def _discover_mcp_background() -> None:
token = set_hermes_home_override(home_override)
try:
from tools.mcp_tool import discover_mcp_tools
discover_mcp_tools()
except Exception:
logger.warning("Background MCP tool discovery failed", exc_info=True)
finally:
reset_hermes_home_override(token)
import threading as _mcp_threading
_mcp_discovery_thread = _mcp_threading.Thread(
target=_discover_mcp_background,
name="tui-mcp-discovery",
daemon=True,
)
_mcp_discovery_thread.start()
start_background_mcp_discovery(
logger=logger, thread_name="tui-mcp-discovery"
)
except Exception:
logger.warning(
"Background MCP tool discovery failed to start", exc_info=True
)
def main():
_install_sidecar_publisher()
# MCP tool discovery — backgrounded so a slow or unreachable MCP server
# can't freeze TUI startup (a dead stdio/http server burns 1+2+4s of
# connect retries → ~7s of dead air before the composer appears). The
# agent isn't built until the first prompt, at which point _make_agent
# briefly joins the discovery thread (wait_for_mcp_discovery, bounded) so
# already-spawning fast servers land in the tool snapshot. The config
# gate inside ensure_mcp_discovery_started keeps the ~200ms MCP SDK
# import cost entirely off the path for users with no mcp_servers.
ensure_mcp_discovery_started()
if not write_json({
"jsonrpc": "2.0",