perf(cold-start): mitigate ~14s GIL stall during backend init (#60800)

hermes_cli/copilot_auth.py

@@ -79,9 +79,11 @@ def resolve_copilot_token() -> tuple[str, str]:
     Raises ValueError if only a classic PAT is available.
     """
     # 1. Check env vars in priority order
+    any_env_var_set = False
     for env_var in COPILOT_ENV_VARS:
         val = os.getenv(env_var, "").strip()
         if val:
+            any_env_var_set = True
             valid, msg = validate_copilot_token(val)
             if not valid:
                 logger.warning(
@@ -90,7 +92,18 @@ def resolve_copilot_token() -> tuple[str, str]:
                 continue
             return val, env_var
 
-    # 2. Fall back to gh auth token
+    # 2. Fall back to gh auth token — but ONLY when no Copilot env var was
+    #    explicitly set. When the user exported GITHUB_TOKEN (even an
+    #    unsupported classic PAT), their intent is to use *that* token, not
+    #    to silently substitute one from the gh CLI credential store.
+    #    Skipping the subprocess here also avoids a slow `gh auth token`
+    #    call (up to 5s timeout on Windows) on every cold start that scans
+    #    Copilot auth state — a measurable contributor to the ~14s
+    #    cold-start stall (#60800). The user can run `copilot login` or
+    #    set a supported token (gho_*/github_pat_*/ghu_) explicitly.
+    if any_env_var_set:
+        return "", ""
+
     token = _try_gh_cli_token()
     if token:
         valid, msg = validate_copilot_token(token)

hermes_cli/web_server.py

@@ -151,10 +151,40 @@ def _start_desktop_cron_ticker(stop_event: "threading.Event", interval: int = 60
 
 
 def _warm_gateway_module() -> None:
-    try:
-        import hermes_cli.gateway  # noqa: F401
-    except Exception:
-        pass
+    """Pre-import heavy modules so the event loop is not stalled on first use.
+
+    On a cold Windows install, importing these module chains triggers .pyc
+    compilation and Defender real-time scans that can stall the event loop
+    for 15-30s. The original fix (pre-#60800) only warmed
+    ``hermes_cli.gateway``. But the first WS connection and its initial
+    RPC burst (``setup.status``, ``setup.runtime_check``,
+    ``gateway.ready``→``resolve_skin``) pull in several *other* heavy
+    chains that were still imported on the loop thread, contributing to
+    the ~14s cold-start stall (#60800). Warm them all here so the cost
+    is paid in a worker thread while the server socket is already open.
+    """
+    for mod in (
+        "hermes_cli.gateway",
+        # setup.status / setup.runtime_check resolve provider auth state,
+        # which imports copilot_auth (→ subprocess module) and scans
+        # credential files. First import is noticeably slow on Windows.
+        "hermes_cli.auth",
+        "hermes_cli.copilot_auth",
+        "hermes_cli.runtime_provider",
+        # resolve_skin() reads config + initialises the skin engine.
+        # Even though handle_ws now calls it via asyncio.to_thread
+        # (see tui_gateway/ws.py), warming it here avoids the first-call
+        # import cost inside that thread.
+        "hermes_cli.skin_engine",
+        # model.options / picker context — parses provider catalogs and
+        # the models.dev cache on first use.
+        "hermes_cli.inventory",
+        "hermes_cli.model_switch",
+    ):
+        try:
+            __import__(mod)
+        except Exception:
+            pass
 
 
 def _resolve_restart_drain_timeout() -> float:

tests/hermes_cli/test_copilot_auth.py

@@ -106,6 +106,37 @@ def test_no_token_returns_empty(self, monkeypatch):
         assert token == ""
         assert source == ""
 
+    def test_invalid_env_var_skips_gh_cli_fallback(self, monkeypatch):
+        """When an env var is set but holds an unsupported classic PAT,
+        resolve_copilot_token must NOT fall back to ``gh auth token``.
+
+        The user explicitly exported a token; silently substituting one
+        from the gh CLI credential store is surprising and the subprocess
+        call adds up to 5s of latency on Windows cold starts (#60800).
+        Only fall back to the CLI when NO Copilot env var is set at all.
+        """
+        from hermes_cli.copilot_auth import resolve_copilot_token
+        monkeypatch.delenv("COPILOT_GITHUB_TOKEN", raising=False)
+        monkeypatch.delenv("GH_TOKEN", raising=False)
+        monkeypatch.setenv("GITHUB_TOKEN", "ghp_classic_pat_nope")
+        with patch("hermes_cli.copilot_auth._try_gh_cli_token") as mock_cli:
+            token, source = resolve_copilot_token()
+        assert token == ""
+        assert source == ""
+        mock_cli.assert_not_called()
+
+    def test_all_env_vars_invalid_skips_gh_cli_fallback(self, monkeypatch):
+        """All three env vars set to classic PATs → no gh CLI call."""
+        from hermes_cli.copilot_auth import resolve_copilot_token
+        monkeypatch.setenv("COPILOT_GITHUB_TOKEN", "ghp_one")
+        monkeypatch.setenv("GH_TOKEN", "ghp_two")
+        monkeypatch.setenv("GITHUB_TOKEN", "ghp_three")
+        with patch("hermes_cli.copilot_auth._try_gh_cli_token") as mock_cli:
+            token, source = resolve_copilot_token()
+        assert token == ""
+        assert source == ""
+        mock_cli.assert_not_called()
+
 
 class TestRequestHeaders:
     """Copilot API header generation."""

tests/tui_gateway/test_cold_start_gil_stall.py

@@ -0,0 +1,144 @@
+"""Tests for cold-start GIL stall mitigations (#60800).
+
+The Desktop/TUI cold start could stall the event loop for ~14s because
+synchronous CPU-bound work ran on the loop thread during the window
+between ``HERMES_BACKEND_READY`` and the first prompt. Three fixes:
+
+1. ``copilot_auth.resolve_copilot_token`` skips the ``gh auth token``
+   subprocess when a Copilot env var is explicitly set (even if invalid).
+2. ``tui_gateway.ws.handle_ws`` runs ``resolve_skin()`` via
+   ``asyncio.to_thread`` so the loop is not blocked by config/skin init.
+3. ``web_server._warm_gateway_module`` pre-imports the heavy module
+   chains that the first WS connection + RPC burst would otherwise
+   import on the loop thread.
+"""
+
+import asyncio
+import inspect
+import sys
+from unittest.mock import patch, MagicMock
+
+import pytest
+
+
+# ─── Fix 1: copilot_auth skips gh CLI when env var is set ──────────────
+
+
+class TestCopilotAuthSkipsGhCli:
+    """resolve_copilot_token must not call _try_gh_cli_token when any
+    Copilot env var is set, even if the token is an unsupported classic PAT.
+
+    See test_copilot_auth.py::TestResolveToken for the full env-var-priority
+    suite; these tests focus on the #60800 cold-start regression — the
+    gh CLI subprocess adds up to 5s on Windows and should not fire when
+    the user already expressed token intent via an env var.
+    """
+
+    def test_invalid_env_var_skips_gh_cli(self, monkeypatch):
+        from hermes_cli.copilot_auth import resolve_copilot_token
+
+        monkeypatch.delenv("COPILOT_GITHUB_TOKEN", raising=False)
+        monkeypatch.delenv("GH_TOKEN", raising=False)
+        monkeypatch.setenv("GITHUB_TOKEN", "ghp_classic_pat_nope")
+        with patch("hermes_cli.copilot_auth._try_gh_cli_token") as mock_cli:
+            token, source = resolve_copilot_token()
+        assert token == ""
+        assert source == ""
+        mock_cli.assert_not_called()
+
+    def test_valid_env_var_skips_gh_cli(self, monkeypatch):
+        """A valid token in an env var should return immediately — no CLI."""
+        from hermes_cli.copilot_auth import resolve_copilot_token
+
+        monkeypatch.setenv("GITHUB_TOKEN", "gho_valid_oauth_token")
+        with patch("hermes_cli.copilot_auth._try_gh_cli_token") as mock_cli:
+            token, source = resolve_copilot_token()
+        assert token == "gho_valid_oauth_token"
+        assert source == "GITHUB_TOKEN"
+        mock_cli.assert_not_called()
+
+    def test_no_env_vars_falls_back_to_gh_cli(self, monkeypatch):
+        """When NO env var is set, the gh CLI fallback must still fire."""
+        from hermes_cli.copilot_auth import resolve_copilot_token
+
+        monkeypatch.delenv("COPILOT_GITHUB_TOKEN", raising=False)
+        monkeypatch.delenv("GH_TOKEN", raising=False)
+        monkeypatch.delenv("GITHUB_TOKEN", raising=False)
+        with patch(
+            "hermes_cli.copilot_auth._try_gh_cli_token",
+            return_value="gho_from_cli",
+        ) as mock_cli:
+            token, source = resolve_copilot_token()
+        assert token == "gho_from_cli"
+        assert source == "gh auth token"
+        mock_cli.assert_called_once()
+
+
+# ─── Fix 2: resolve_skin runs via to_thread in handle_ws ───────────────
+
+
+def test_handle_ws_uses_to_thread_for_resolve_skin():
+    """handle_ws must call resolve_skin through asyncio.to_thread, not
+    inline on the event loop thread (#60800).
+
+    We verify by inspecting the source of handle_ws — the call to
+    ``server.resolve_skin`` must be wrapped in ``asyncio.to_thread``.
+    A regression that reverts to inline ``resolve_skin()`` would fail
+    this assertion.
+    """
+    import tui_gateway.ws as ws_mod
+
+    source = inspect.getsource(ws_mod.handle_ws)
+    assert "asyncio.to_thread" in source, (
+        "handle_ws must call resolve_skin via asyncio.to_thread to avoid "
+        "blocking the event loop during cold start (#60800)."
+    )
+    assert "resolve_skin" in source
+
+
+# ─── Fix 3: _warm_gateway_module pre-imports heavy chains ──────────────
+
+
+def test_warm_gateway_module_imports_cold_start_chains():
+    """_warm_gateway_module must pre-import the module chains that the
+    first WS connection + RPC burst would otherwise import on the loop
+    thread (#60800). Each of these chains involves .pyc compilation,
+    Defender scans, or heavy transitive imports that stall the loop.
+
+    We verify by patching __import__ to record which modules were
+    requested, then assert the cold-start-critical modules are present.
+    """
+    import hermes_cli.web_server as web_server_mod
+
+    # The set of modules that MUST be warmed — these are imported on the
+    # first WS connection / RPC burst and are heavy enough to stall the
+    # loop on Windows cold starts.
+    required = {
+        "hermes_cli.gateway",
+        "hermes_cli.auth",
+        "hermes_cli.copilot_auth",
+        "hermes_cli.runtime_provider",
+        "hermes_cli.skin_engine",
+        "hermes_cli.inventory",
+        "hermes_cli.model_switch",
+    }
+
+    imported = []
+    real_import = __import__
+
+    def tracking_import(name, *args, **kwargs):
+        imported.append(name)
+        # Don't actually import — we only care about what was requested.
+        # Raise ImportError to let _warm_gateway_module's except pass.
+        raise ImportError(f"tracking stub for {name}")
+
+    with patch("builtins.__import__", tracking_import):
+        web_server_mod._warm_gateway_module()
+
+    imported_set = set(imported)
+    missing = required - imported_set
+    assert not missing, (
+        f"_warm_gateway_module did not pre-import cold-start-critical "
+        f"modules: {missing}. These must be warmed in a background thread "
+        f"to avoid stalling the event loop (#60800)."
+    )

tui_gateway/ws.py

@@ -316,13 +316,21 @@ async def handle_ws(ws: Any) -> None:
             thread_name="tui-ws-mcp-discovery",
         )
 
+        # resolve_skin() reads config + initializes the skin engine —
+        # synchronous I/O + CPU work that should not block the event loop
+        # during the cold-start window. Run it in the thread pool so the
+        # WS read loop stays free to drain the frontend's initial RPC
+        # burst (setup.status, session.list, ...) without a stall
+        # (#60800). The skin payload is small (a dict of strings/arrays),
+        # so the to_thread overhead is negligible.
+        skin_payload = await asyncio.to_thread(server.resolve_skin)
         ready_ok = await transport.write_async(
             {
                 "jsonrpc": "2.0",
                 "method": "event",
                 "params": {
                     "type": "gateway.ready",
-                    "payload": {"skin": server.resolve_skin()},
+                    "payload": {"skin": skin_payload},
                 },
             }
         )