From 98b6f8676d26544da4ba655a6602effbf92e9721 Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Wed, 19 Aug 2026 16:39:49 -0700
Subject: [PATCH] fix(desktop): Models/Providers settings no longer hang 20s
 when gh CLI is signed out
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

/api/model/options probed Copilot auth via `gh auth token` four separate
times per payload build. When gh has no credential store for the backend's
HOME (fresh profile, desktop-spawned backend, CI), each probe blocks its
full 5s subprocess timeout on keyring/D-Bus, so every open of the Desktop
Models or Providers settings page took 20s — past the renderer's 15s IPC
budget, painting 'Error invoking remote method hermes:api: Timed out'.

Fix: cache the gh-CLI probe result (hit or miss) for 5 minutes with an
invalidation hook, feed gh stdin=DEVNULL, and disable gh interactive
prompts/update notifier in the probe env.

Measured on the failing profile: 20.5s -> 5.3s cold (one bounded probe),
0.03s warm.
---
 hermes_cli/copilot_auth.py                 | 43 +++++++++++++++++
 tests/hermes_cli/test_api_key_providers.py |  3 ++
 tests/hermes_cli/test_copilot_auth.py      | 55 ++++++++++++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py
index 8416f99d22..e851db5b2e 100644
--- a/hermes_cli/copilot_auth.py
+++ b/hermes_cli/copilot_auth.py
@@ -142,6 +142,26 @@ def _gh_cli_candidates() -> list[str]:
     return candidates
 
 
+# ``gh auth token`` result cache. The probe shells out to the gh CLI, and when
+# gh has no credential store for this HOME (fresh profile, desktop-spawned
+# backend, CI) it can block for its full 5s subprocess timeout — on keyring /
+# D-Bus prompts rather than returning immediately. Provider inventory builds
+# (``/api/model/options``, ``hermes tools``) probe Copilot auth several times
+# per request, so an uncached miss turns one settings-page load into a 4×5s
+# stall that exceeds the Desktop renderer's 15s IPC budget and paints an error
+# (observed Aug 2026: Models/Providers settings pages timing out on every
+# open). Successes and failures are both cached; a short TTL keeps a freshly
+# run ``gh auth login`` discoverable without restarting the backend.
+_GH_CLI_TOKEN_CACHE_TTL_SECONDS = 300.0
+_gh_cli_token_cache: tuple[float, Optional[str]] | None = None
+
+
+def _invalidate_gh_cli_token_cache() -> None:
+    """Reset the ``gh auth token`` probe cache (used by tests and re-auth flows)."""
+    global _gh_cli_token_cache
+    _gh_cli_token_cache = None
+
+
 def _try_gh_cli_token() -> Optional[str]:
     """Return a token from ``gh auth token`` when the GitHub CLI is available.
 
@@ -149,12 +169,34 @@ def _try_gh_cli_token() -> Optional[str]:
     correct host's token.  Also strips GITHUB_TOKEN / GH_TOKEN from the
     subprocess environment so ``gh`` reads from its own credential store
     (hosts.yml) instead of just echoing the env var back.
+
+    The result (including a miss) is cached for a short TTL — see the cache
+    comment above. Callers that just re-authenticated can call
+    ``_invalidate_gh_cli_token_cache()`` to re-probe immediately.
     """
+    global _gh_cli_token_cache
+
+    now = time.monotonic()
+    if _gh_cli_token_cache is not None:
+        cached_at, cached_token = _gh_cli_token_cache
+        if now - cached_at < _GH_CLI_TOKEN_CACHE_TTL_SECONDS:
+            return cached_token
+
+    token = _probe_gh_cli_token()
+    _gh_cli_token_cache = (now, token)
+    return token
+
+
+def _probe_gh_cli_token() -> Optional[str]:
+    """Uncached ``gh auth token`` subprocess probe (see ``_try_gh_cli_token``)."""
     hostname = os.getenv("COPILOT_GH_HOST", "").strip()
 
     # Build a clean env so gh doesn't short-circuit on GITHUB_TOKEN / GH_TOKEN
     clean_env = {k: v for k, v in os.environ.items()
                  if k not in {"GITHUB_TOKEN", "GH_TOKEN"}}
+    # Never let gh open an interactive prompt from a backend process.
+    clean_env.setdefault("GH_PROMPT_DISABLED", "1")
+    clean_env.setdefault("GH_NO_UPDATE_NOTIFIER", "1")
 
     _popen_kwargs = {"creationflags": windows_hide_flags()} if IS_WINDOWS else {}
     for gh_path in _gh_cli_candidates():
@@ -168,6 +210,7 @@ def _try_gh_cli_token() -> Optional[str]:
                 text=True, encoding='utf-8', errors='replace',
                 timeout=5,
                 env=clean_env,
+                stdin=subprocess.DEVNULL,
                 **_popen_kwargs,
             )
         except (FileNotFoundError, subprocess.TimeoutExpired) as exc:
diff --git a/tests/hermes_cli/test_api_key_providers.py b/tests/hermes_cli/test_api_key_providers.py
index 619cc74c45..a6dc235113 100644
--- a/tests/hermes_cli/test_api_key_providers.py
+++ b/tests/hermes_cli/test_api_key_providers.py
@@ -341,6 +341,9 @@ class TestResolveApiKeyProviderCredentials:
 
 
     def test_try_gh_cli_token_uses_homebrew_path_when_not_on_path(self, monkeypatch):
+        from hermes_cli.copilot_auth import _invalidate_gh_cli_token_cache
+
+        _invalidate_gh_cli_token_cache()
         monkeypatch.setattr("hermes_cli.copilot_auth.shutil.which", lambda command: None)
         monkeypatch.setattr(
             "hermes_cli.copilot_auth.os.path.isfile",
diff --git a/tests/hermes_cli/test_copilot_auth.py b/tests/hermes_cli/test_copilot_auth.py
index 1023bbfa95..334f1eb2e4 100644
--- a/tests/hermes_cli/test_copilot_auth.py
+++ b/tests/hermes_cli/test_copilot_auth.py
@@ -72,6 +72,61 @@ class TestResolveToken:
         mock_cli.assert_not_called()
 
 
+class TestGhCliTokenCache:
+    """The gh-CLI probe result is cached — a miss must not re-spawn gh.
+
+    Regression: /api/model/options ran `gh auth token` four times per build;
+    with no gh credential store each probe blocked its full 5s timeout, so the
+    Desktop Models/Providers settings pages took 20s per open and exceeded the
+    renderer's 15s IPC budget (Aug 2026 desktop audit).
+    """
+
+    def _reset(self):
+        from hermes_cli.copilot_auth import _invalidate_gh_cli_token_cache
+        _invalidate_gh_cli_token_cache()
+
+    def test_miss_is_cached_and_probe_runs_once(self):
+        from hermes_cli import copilot_auth
+        self._reset()
+        with patch.object(copilot_auth, "_probe_gh_cli_token", return_value=None) as probe:
+            assert copilot_auth._try_gh_cli_token() is None
+            assert copilot_auth._try_gh_cli_token() is None
+            assert copilot_auth._try_gh_cli_token() is None
+        assert probe.call_count == 1
+        self._reset()
+
+    def test_hit_is_cached(self):
+        from hermes_cli import copilot_auth
+        self._reset()
+        with patch.object(copilot_auth, "_probe_gh_cli_token", return_value="gho_cached") as probe:
+            assert copilot_auth._try_gh_cli_token() == "gho_cached"
+            assert copilot_auth._try_gh_cli_token() == "gho_cached"
+        assert probe.call_count == 1
+        self._reset()
+
+    def test_ttl_expiry_reprobes(self, monkeypatch):
+        from hermes_cli import copilot_auth
+        self._reset()
+        clock = {"now": 1000.0}
+        monkeypatch.setattr(copilot_auth.time, "monotonic", lambda: clock["now"])
+        with patch.object(copilot_auth, "_probe_gh_cli_token", return_value=None) as probe:
+            copilot_auth._try_gh_cli_token()
+            clock["now"] += copilot_auth._GH_CLI_TOKEN_CACHE_TTL_SECONDS + 1
+            copilot_auth._try_gh_cli_token()
+        assert probe.call_count == 2
+        self._reset()
+
+    def test_invalidate_forces_reprobe(self):
+        from hermes_cli import copilot_auth
+        self._reset()
+        with patch.object(copilot_auth, "_probe_gh_cli_token", return_value=None) as probe:
+            copilot_auth._try_gh_cli_token()
+            copilot_auth._invalidate_gh_cli_token_cache()
+            copilot_auth._try_gh_cli_token()
+        assert probe.call_count == 2
+        self._reset()
+
+
 class TestRequestHeaders:
     """Copilot API header generation."""