fix(oneshot): join MCP discovery before the one-shot tool snapshot (bounded, gated, warns)

hermes_cli/oneshot.py

@@ -50,6 +50,106 @@ def _normalize_toolsets(toolsets: object = None) -> list[str] | None:
     return [item for item in normalized if item] or None
 
 
+def _read_mcp_server_names() -> tuple[set[str], set[str]]:
+    """Return (enabled, disabled) MCP server names from config.yaml.
+
+    Shared by the ``--toolsets`` resolver and the one-shot MCP discovery wait so
+    both classify configured servers identically. Fail-safe: any config error
+    yields empty sets rather than raising.
+    """
+    enabled: set[str] = set()
+    disabled: set[str] = set()
+    try:
+        from hermes_cli.config import read_raw_config
+        from hermes_cli.tools_config import _parse_enabled_flag
+
+        cfg = read_raw_config()
+        mcp_servers = cfg.get("mcp_servers") if isinstance(cfg.get("mcp_servers"), dict) else {}
+        for name, server_cfg in mcp_servers.items():
+            if not isinstance(server_cfg, dict):
+                continue
+            if _parse_enabled_flag(server_cfg.get("enabled", True), default=True):
+                enabled.add(str(name))
+            else:
+                disabled.add(str(name))
+    except Exception:
+        return set(), set()
+    return enabled, disabled
+
+
+def _effective_oneshot_mcp_servers(
+    toolsets: object = None,
+    *,
+    use_config_toolsets: bool = True,
+) -> set[str]:
+    """Return configured MCP servers that can contribute to this invocation."""
+    enabled, _disabled = _read_mcp_server_names()
+    if not enabled:
+        return set()
+
+    normalized = _normalize_toolsets(toolsets)
+    if not use_config_toolsets:
+        # ``None`` on this path is the validated representation of explicit
+        # ``--toolsets all`` / ``*``. Every enabled MCP server can contribute.
+        return enabled if normalized is None else enabled.intersection(normalized)
+
+    try:
+        from hermes_cli.config import load_config
+        from hermes_cli.tools_config import _get_platform_tools
+
+        effective_toolsets = _get_platform_tools(load_config(), "cli")
+    except Exception:
+        # Match the startup path's fail-safe posture: if effective toolset
+        # resolution breaks, do not reintroduce the original discovery race.
+        return enabled
+    return enabled.intersection(effective_toolsets)
+
+
+def _wait_for_mcp_discovery_before_snapshot(
+    toolsets: object = None,
+    *,
+    use_config_toolsets: bool = True,
+) -> None:
+    """Join background MCP discovery before the one-shot tool snapshot is built.
+
+    ``hermes -z`` starts MCP discovery on a background thread (via
+    ``_prepare_agent_startup``) but, unlike the interactive CLI/TUI, never joined
+    it — so a configured server that had not finished connecting was silently
+    absent from the turn's tools. Wait here, bounded like the TUI's late-refresh
+    join, whenever an enabled MCP server is in this invocation's effective
+    toolset. Zero cost when MCP is excluded, and ~0s when discovery already
+    finished (``join`` returns the instant the thread is done). Servers still
+    pending after the configured bound are named on stderr; one-shot proceeds
+    with the tools that finished discovery within the bound.
+
+    Must run BEFORE ``run_oneshot`` redirects stderr, so the warning reaches the
+    terminal (mirrors the ``--toolsets`` warnings above).
+    """
+    selected_mcp_servers = _effective_oneshot_mcp_servers(
+        toolsets,
+        use_config_toolsets=use_config_toolsets,
+    )
+    if not selected_mcp_servers:
+        return
+    try:
+        from hermes_cli.mcp_startup import (
+            _resolve_discovery_timeout,
+            mcp_discovery_in_flight,
+            wait_for_mcp_discovery,
+        )
+    except Exception:
+        return
+
+    timeout = _resolve_discovery_timeout(None)
+    wait_for_mcp_discovery(timeout=timeout)
+    if mcp_discovery_in_flight():
+        sys.stderr.write(
+            f"hermes -z: MCP discovery still pending after {timeout:g}s; "
+            "tools from these servers may be missing this turn: "
+            f"{', '.join(sorted(selected_mcp_servers))}\n"
+        )
+
+
 def _validate_explicit_toolsets(toolsets: object = None) -> tuple[list[str] | None, str | None]:
     normalized = _normalize_toolsets(toolsets)
     if normalized is None:
@@ -88,22 +188,7 @@ def _validate_explicit_toolsets(toolsets: object = None) -> tuple[list[str] | No
     mcp_names: set[str] = set()
     mcp_disabled: set[str] = set()
     if unresolved:
-        try:
-            from hermes_cli.config import read_raw_config
-            from hermes_cli.tools_config import _parse_enabled_flag
-
-            cfg = read_raw_config()
-            mcp_servers = cfg.get("mcp_servers") if isinstance(cfg.get("mcp_servers"), dict) else {}
-            for name, server_cfg in mcp_servers.items():
-                if not isinstance(server_cfg, dict):
-                    continue
-                if _parse_enabled_flag(server_cfg.get("enabled", True), default=True):
-                    mcp_names.add(str(name))
-                else:
-                    mcp_disabled.add(str(name))
-        except Exception:
-            mcp_names = set()
-            mcp_disabled = set()
+        mcp_names, mcp_disabled = _read_mcp_server_names()
 
     mcp_valid = [name for name in unresolved if name in mcp_names]
     disabled = [name for name in unresolved if name in mcp_disabled]
@@ -216,6 +301,15 @@ def run_oneshot(
         return 2
     use_config_toolsets = _normalize_toolsets(toolsets) is None
 
+    # Join background MCP discovery before the agent's tool snapshot is built.
+    # Done here (before the stderr redirect below) so any pending-server warning
+    # reaches the terminal, and so a slow MCP server's tools are actually present
+    # for this single turn — one-shot has no interactive late-binding refresh.
+    _wait_for_mcp_discovery_before_snapshot(
+        explicit_toolsets,
+        use_config_toolsets=use_config_toolsets,
+    )
+
     # Auto-approve any shell / tool approvals.  Non-interactive by
     # definition — a prompt would hang forever.
     os.environ["HERMES_YOLO_MODE"] = "1"

tests/hermes_cli/test_oneshot_mcp_wait.py

@@ -0,0 +1,159 @@
+"""Tests for hermes -z joining background MCP discovery before the tool snapshot.
+
+Regression: one-shot started MCP discovery on a background thread but never
+joined it, so a still-connecting server's tools were silently absent from the
+turn. ``_wait_for_mcp_discovery_before_snapshot`` now bounds-joins that thread.
+"""
+
+from hermes_cli import mcp_startup
+from hermes_cli import oneshot as oneshot_mod
+
+
+def test_waits_with_bounded_timeout_when_mcp_configured(monkeypatch):
+    monkeypatch.setattr(oneshot_mod, "_read_mcp_server_names", lambda: ({"demo"}, set()))
+    calls: dict = {}
+    monkeypatch.setattr(mcp_startup, "_resolve_discovery_timeout", lambda _explicit: 1.5)
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: calls.__setitem__("timeout", timeout),
+    )
+    monkeypatch.setattr(mcp_startup, "mcp_discovery_in_flight", lambda: False)
+
+    oneshot_mod._wait_for_mcp_discovery_before_snapshot(
+        ["demo"],
+        use_config_toolsets=False,
+    )
+
+    assert calls["timeout"] == 1.5
+
+
+def test_no_wait_when_no_mcp_configured(monkeypatch):
+    monkeypatch.setattr(oneshot_mod, "_read_mcp_server_names", lambda: (set(), set()))
+    called = {"n": 0}
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: called.__setitem__("n", called["n"] + 1),
+    )
+
+    oneshot_mod._wait_for_mcp_discovery_before_snapshot()
+
+    # Zero cost for the common no-MCP path — discovery is never joined.
+    assert called["n"] == 0
+
+
+def test_timeout_warns_and_proceeds(monkeypatch, capsys):
+    monkeypatch.setattr(
+        oneshot_mod, "_read_mcp_server_names", lambda: ({"demo", "other"}, set())
+    )
+    monkeypatch.setattr(mcp_startup, "_resolve_discovery_timeout", lambda _explicit: 2.75)
+    monkeypatch.setattr(mcp_startup, "wait_for_mcp_discovery", lambda timeout=None: None)
+    # Still in flight after the bound == discovery timed out.
+    monkeypatch.setattr(mcp_startup, "mcp_discovery_in_flight", lambda: True)
+
+    # Must not raise — the run proceeds with whatever tools did connect.
+    oneshot_mod._wait_for_mcp_discovery_before_snapshot(
+        ["demo", "other"],
+        use_config_toolsets=False,
+    )
+
+    err = capsys.readouterr().err
+    assert err == (
+        "hermes -z: MCP discovery still pending after 2.75s; tools from these "
+        "servers may be missing this turn: demo, other\n"
+    )
+
+
+def test_explicit_non_mcp_toolsets_skip_wait(monkeypatch, capsys):
+    monkeypatch.setattr(oneshot_mod, "_read_mcp_server_names", lambda: ({"demo"}, set()))
+    calls = {"wait": 0}
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: calls.__setitem__("wait", calls["wait"] + 1),
+    )
+    monkeypatch.setattr(oneshot_mod, "_run_agent", lambda *_args, **_kwargs: ("done", {}))
+
+    assert oneshot_mod.run_oneshot("hello", toolsets="web,terminal") == 0
+
+    assert calls["wait"] == 0
+    assert capsys.readouterr().out == "done\n"
+
+
+def test_no_mcp_sentinel_in_config_skips_wait(monkeypatch, capsys):
+    from hermes_cli import config as config_mod
+
+    config = {
+        "mcp_servers": {"demo": {"command": "demo-server"}},
+        "platform_toolsets": {"cli": ["web", "no_mcp"]},
+    }
+    monkeypatch.setattr(oneshot_mod, "_read_mcp_server_names", lambda: ({"demo"}, set()))
+    monkeypatch.setattr(config_mod, "load_config", lambda: config)
+    calls = {"wait": 0}
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: calls.__setitem__("wait", calls["wait"] + 1),
+    )
+    monkeypatch.setattr(oneshot_mod, "_run_agent", lambda *_args, **_kwargs: ("done", {}))
+
+    assert oneshot_mod.run_oneshot("hello") == 0
+
+    assert calls["wait"] == 0
+    assert capsys.readouterr().out == "done\n"
+
+
+def test_config_resolved_timeout_reaches_wait_call(monkeypatch):
+    from hermes_cli import config as config_mod
+
+    monkeypatch.setattr(oneshot_mod, "_read_mcp_server_names", lambda: ({"demo"}, set()))
+    monkeypatch.setattr(config_mod, "load_config", lambda: {"mcp_discovery_timeout": 7.25})
+    calls: dict = {}
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: calls.__setitem__("timeout", timeout),
+    )
+    monkeypatch.setattr(mcp_startup, "mcp_discovery_in_flight", lambda: False)
+
+    oneshot_mod._wait_for_mcp_discovery_before_snapshot(
+        ["demo"],
+        use_config_toolsets=False,
+    )
+
+    assert calls["timeout"] == 7.25
+
+
+def test_temp_hermes_home_waits_before_oneshot_snapshot(monkeypatch, tmp_path, capsys):
+    hermes_home = tmp_path / ".hermes"
+    hermes_home.mkdir()
+    (hermes_home / "config.yaml").write_text(
+        "mcp_servers:\n"
+        "  demo:\n"
+        "    command: demo-server\n"
+        "platform_toolsets:\n"
+        "  cli:\n"
+        "    - web\n"
+        "    - demo\n",
+        encoding="utf-8",
+    )
+    monkeypatch.setenv("HERMES_HOME", str(hermes_home))
+
+    events: list[str] = []
+    monkeypatch.setattr(
+        mcp_startup,
+        "wait_for_mcp_discovery",
+        lambda timeout=None: events.append("wait"),
+    )
+    monkeypatch.setattr(mcp_startup, "mcp_discovery_in_flight", lambda: False)
+
+    def _snapshot(*_args, **_kwargs):
+        events.append("snapshot")
+        return "done", {}
+
+    monkeypatch.setattr(oneshot_mod, "_run_agent", _snapshot)
+
+    assert oneshot_mod.run_oneshot("hello") == 0
+    assert events == ["wait", "snapshot"]
+    assert capsys.readouterr().out == "done\n"