diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index 18d8f0ba37..fca2f96b70 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -24,10 +24,9 @@ from hermes_cli._subprocess_compat import IS_WINDOWS, windows_hide_flags logger = logging.getLogger(__name__) -# OAuth device code flow — VS Code's GitHub App client ID. The opencode OAuth App ID -# (Ov23li8tweQw6odWQebz) produces gho_* tokens that cannot be exchanged for Copilot API JWTs -# (404 on /copilot_internal/v2/token); VS Code's produces ghu_* tokens that support exchange, -# required for internal-only models and enterprise endpoints. Tested on Individual + Enterprise. +# OAuth device code flow — VS Code's GitHub App client ID: it mints ghu_* tokens that can be +# exchanged for Copilot API JWTs (required for internal-only models and enterprise endpoints). +# The opencode App ID (Ov23li8tweQw6odWQebz) mints gho_* tokens that 404 on exchange. COPILOT_OAUTH_CLIENT_ID = "Iv1.b507a08c87ecfe98" # ghp_ classic PATs are rejected by the Copilot API (gho_ / github_pat_ / ghu_ work). _CLASSIC_PAT_PREFIX = "ghp_" @@ -105,12 +104,11 @@ def _gh_cli_candidates() -> list[str]: return candidates -# ``gh auth token`` result cache. When gh has no credential store for this HOME (fresh -# profile, desktop-spawned backend, CI) the probe can block for its full 5s timeout on -# keyring / D-Bus prompts. Provider inventory builds (``/api/model/options``, ``hermes tools``) -# probe Copilot auth several times per request, so an uncached miss turned one settings-page -# load into a 4×5s stall exceeding the Desktop renderer's 15s IPC budget. Successes and -# failures are both cached; a short TTL keeps a fresh ``gh auth login`` discoverable. +# ``gh auth token`` result cache. With no credential store for this HOME (fresh profile, CI) +# the probe blocks for its full 5s timeout on keyring / D-Bus prompts, and provider inventory +# builds probe Copilot auth several times per request — an uncached miss made one settings +# page a 4×5s stall past the Desktop renderer's 15s IPC budget. Misses are cached too; a short +# TTL keeps a fresh ``gh auth login`` discoverable. _GH_CLI_TOKEN_CACHE_TTL_SECONDS = 300.0 _gh_cli_token_cache: tuple[float, Optional[str]] | None = None @@ -264,27 +262,24 @@ _TOKEN_EXCHANGE_URL = "https://api.github.com/copilot_internal/v2/token" _EDITOR_VERSION = "vscode/1.104.1" _EXCHANGE_USER_AGENT = "GitHubCopilotChat/0.26.7" -# Transient-failure hardening. Gateway startup often races network readiness (launchd -# relaunch, DHCP/VPN settling); a single-shot exchange that fails there silently degrades to -# the RAW GitHub token, which the Copilot server routes to the "copilot-language-server" -# integrator whose model allowlist omits enterprise-only models → HTTP 400 on every turn -# until the next restart. Retry a few times, and persist the last good exchanged JWT so a -# restart during a blip reuses the still-valid ~30-min token instead of degrading. +# Transient-failure hardening. Gateway startup races network readiness (launchd relaunch, +# DHCP/VPN settling); a single-shot exchange failing there silently degrades to the RAW GitHub +# token, which Copilot routes to the "copilot-language-server" integrator whose allowlist omits +# enterprise-only models → HTTP 400 every turn until restart. Retry, and persist the last good +# JWT so a restart during a blip reuses the still-valid ~30-min token. _EXCHANGE_MAX_ATTEMPTS = 3 _EXCHANGE_BACKOFF_BASE_SECONDS = 1.5 # sleeps ~1.5s, ~3.0s between attempts _JWT_DISK_FILENAME = ".copilot_jwt.json" _JWT_DISK_MAX_BYTES = 1_048_576 # 1 MiB cap on the persisted JWT store read -# Negative cache for failed exchanges: raw-token fingerprint -> epoch until which attempts -# raise immediately (success clears the entry). Without it every load_pool("copilot") re-ran -# the full exchange, and on a permanently-rejected token (403: not Copilot-entitled, expired -# grant, org policy) the retry backoff burned ~4.5s of sleep on EVERY provider-discovery pass -# (/model picker, delegation child spawns, web dashboard). +# Negative cache: fingerprint -> epoch until which attempts raise immediately (success clears +# it). Without it a permanently-rejected token (403: not entitled, expired grant, org policy) +# burned ~4.5s of retry backoff on EVERY provider-discovery pass (/model picker, delegation +# spawns, dashboard). _exchange_failure_cache: dict[str, float] = {} -# Single-flight guard per token fingerprint: concurrent callers (the dashboard polls -# /api/credentials/pool every few seconds, each poll off-loop) wait on the ONE in-flight -# exchange and then hit the positive/negative cache, instead of each spawning their own hung -# resolver thread during a DNS outage. +# Single-flight guard per fingerprint: concurrent callers (dashboard polls /api/credentials/ +# pool every few seconds) wait on the ONE in-flight exchange and then hit the cache, instead +# of each spawning its own hung resolver thread during a DNS outage. _exchange_locks: dict[str, threading.Lock] = {} _exchange_locks_guard = threading.Lock() @@ -411,10 +406,9 @@ def _save_jwt_to_disk(fp: str, api_token: str, expires_at: float, base_url: Opti _with_jwt_store("persist", _save) -# Hard wall-clock cap for the token-exchange HTTP call. urllib's ``timeout`` only bounds -# socket operations AFTER DNS resolution succeeds; getaddrinfo blocks in C and ignores it, -# so on a networkless Windows host the resolver can hang for many minutes (observed: a -# 17-minute event-loop stall that took the whole backend down). +# Hard wall-clock cap for the exchange call: urllib's ``timeout`` only bounds socket ops AFTER +# DNS succeeds; getaddrinfo blocks in C and ignores it, so a networkless Windows host can hang +# for many minutes (observed: a 17-minute event-loop stall that took the backend down). _DNS_GRACE_SECONDS = 5.0 diff --git a/hermes_cli/dashboard_procs.py b/hermes_cli/dashboard_procs.py index 5ec2e96868..afde2e30dc 100644 --- a/hermes_cli/dashboard_procs.py +++ b/hermes_cli/dashboard_procs.py @@ -10,9 +10,8 @@ import subprocess import sys from pathlib import Path -# Cmdline substrings identifying the long-lived server. ``hermes serve`` is the same server -# under the headless name the desktop app spawns; it is reaped on update for the same -# frontend/backend-mismatch reason as ``dashboard``. +# Cmdline substrings identifying the long-lived server; ``hermes serve`` is the same server +# under the headless name the desktop app spawns, reaped on update for the same reason. _DASHBOARD_PATTERNS = tuple( f"{launcher} {cmd}" for cmd in ("dashboard", "serve") @@ -42,10 +41,9 @@ def _iter_process_table() -> list[tuple[int, str]]: """``(pid, cmdline)`` for every process, via wmic (Windows) or ps. Raises on scan failure.""" rows: list[tuple[int, str]] = [] if sys.platform == "win32": - # errors="ignore": wmic may emit the system code page; a decode error would leave - # stdout=None. bounded_probe_run (not run()): run()'s post-timeout cleanup joins pipe - # readers unbounded and a conhost descendant holding duplicated handles wedges it - # forever. It also passes CREATE_NO_WINDOW for the pythonw.exe backend. + # errors="ignore": wmic may emit the system code page (a decode error leaves stdout + # None). bounded_probe_run, not run(): run()'s post-timeout cleanup joins pipe readers + # unbounded and a conhost descendant holding duplicated handles wedges it forever. from hermes_cli._subprocess_compat import bounded_probe_run result = bounded_probe_run( ["wmic", "process", "get", "ProcessId,CommandLine", "/FORMAT:LIST"], @@ -73,13 +71,11 @@ def _iter_process_table() -> list[tuple[int, str]]: def _scan_dashboard_processes(*, exclude_pids: set[int] | None = None) -> list[tuple[int, str]]: - """Return matching ``dashboard``/``serve`` processes with their cmdlines. + """``(pid, cmdline)`` of running ``dashboard``/``serve`` processes; empty on any scan error. - ``hermes update`` swaps files on disk while a forgotten dashboard keeps the old Python - backend in memory against the new JS bundle — a silent mismatch (new auth headers → every - API call 401s). *exclude_pids* must never be returned: Hermes Desktop sets - ``HERMES_DESKTOP_CHILD_PID`` on the backend it spawns so an auto-update never kills the - backend it manages itself. Empty list on any scan error (missing ps/wmic, timeout, ...). + A forgotten dashboard keeps the old Python backend in memory against the new JS bundle + after ``hermes update`` (new auth headers → every API call 401s). *exclude_pids* must never + be returned: Desktop marks the backend it manages via ``HERMES_DESKTOP_CHILD_PID``. """ self_pid = os.getpid() skip = {self_pid, *(exclude_pids or ())} @@ -91,10 +87,8 @@ def _scan_dashboard_processes(*, exclude_pids: set[int] | None = None) -> list[t except (FileNotFoundError, subprocess.TimeoutExpired, OSError): return [] - # Spawn-ledger augmentation: substring patterns miss profiled launches - # (`hermes --profile p serve ...`). Every serve/dashboard registers itself in the spawn - # ledger with live-verified (pid, create_time) — positive identity. Add entries the scan - # missed, preferring the ledger's full argv. + # Spawn-ledger augmentation: substring patterns miss profiled launches (`hermes --profile + # p serve`); the ledger holds live-verified (pid, create_time) — positive identity. try: from hermes_cli.process_identity import ledger_entries seen = {pid for pid, _ in found} | skip @@ -146,10 +140,8 @@ def _profile_flag_value(argv: list[str]) -> str | None: def _is_ephemeral_port_zero_backend(argv: list[str]) -> bool: """True for Desktop-style ``serve|dashboard --port 0`` backends. - Ephemeral-port backends are owned by Hermes Desktop (or are PPID-1 orphans of a prior - update respawn). Replaying them after ``hermes update`` multiplies listening backends - because ``--port 0`` always binds a fresh port. Covers ``serve`` and the legacy - ``dashboard --no-open`` fallback older Desktop runtimes use. + Owned by Hermes Desktop (or PPID-1 orphans of a prior respawn); replaying them after + ``hermes update`` multiplies listening backends because ``--port 0`` binds a fresh port. """ if _dashboard_subcommand_index(argv) is None: return False @@ -210,20 +202,14 @@ def _profile_key_for_respawn(argv: list[str], hermes_home: str | None = None) -> def _filter_dashboard_respawn_candidates( candidates: list[tuple[int, list[str], str | None]], *, own_home: str | None = None ) -> list[list[str]]: - """Select which killed manual backends to respawn after ``hermes update``. + """Select which killed manual backends ``(pid, argv, hermes_home)`` to respawn after update. - Candidates are ``(pid, argv, hermes_home)``; *own_home* (default ``get_hermes_home()``) - is a parameter so tests can pin it. Rules: - 1. Never resurrect Desktop ephemeral ``--port 0`` backends — Desktop owns their lifecycle. - 2. Never replay a backend from a **foreign** ``HERMES_HOME``: the respawn is argv-only - (no ``env=``), so it would come back on the *updating* install's home and steal the - foreign install's fixed port, leaving its supervisor to crash-loop on ``EADDRINUSE``. - Unreadable (``None``) stays eligible. - 3. Dedupe by normalized cmdline. 4. At most one backend per profile / home. - - Does **not** blanket-skip PPID-1: a prior update respawn detaches with - ``start_new_session=True``, so fixed-port manual backends sit under init and must stay - eligible next update. + Rules: never resurrect Desktop ``--port 0`` backends (Desktop owns them); never replay a + backend from a **foreign** ``HERMES_HOME`` (the respawn is argv-only, so it would come back + on the updating install's home and steal the foreign install's fixed port → its supervisor + crash-loops on ``EADDRINUSE``; unreadable ``None`` stays eligible); dedupe by normalized + cmdline; at most one backend per profile / home. PPID-1 is NOT skipped: a prior respawn + detaches with ``start_new_session=True``, so fixed-port manual backends sit under init. """ if own_home is None: try: @@ -334,12 +320,10 @@ def _kill_stale_dashboard_processes( ) -> dict[str, list]: """Kill running ``hermes dashboard`` / ``hermes serve`` processes (update end, ``--stop``). - With ``restart_managed`` (update path only) a detected ``hermes-dashboard.service`` is - restarted through systemd, any other killed PID owned by a systemd unit has that unit - restarted after the kill (systemd treats our SIGTERM as a clean stop, so - ``Restart=on-failure`` never fires), and manual PIDs are respawned from their captured - argv. *already_restarted_units* (no ``.service`` suffix) were restarted by the caller - already; PIDs they own are left untouched, not killed twice. + With ``restart_managed`` (update only) systemd-owned PIDs get their unit restarted after the + kill (systemd treats our SIGTERM as a clean stop, so ``Restart=on-failure`` never fires) and + manual PIDs are respawned from captured argv. PIDs owned by *already_restarted_units* (no + ``.service`` suffix) are left untouched, not killed twice. """ if restart_managed and _m()._restart_managed_dashboard_service(reason): # The dashboard unit is handled but every OTHER backend is not (a host may also run @@ -460,13 +444,11 @@ def _detect_concurrent_hermes_instances( ) -> list[tuple[int, str]]: """``(pid, name)`` of other live processes whose .exe is one of our entry-point shims. - Windows blocks DELETE/REPLACE on a running .exe; Desktop spawns ``hermes.EXE`` as a backend - child, so the update's quarantine rename fails with ``[WinError 32]``. Excludes our own - PID and every *shim* ancestor (the setuptools launcher is a separate native process from - the ``python.exe`` it loads — otherwise every update reports its own launcher); a second - hermes.exe under a non-Hermes parent (Desktop child) is still flagged. ``proc.parents()`` - (whole chain at once) because a per-hop loop bailed on the first AccessDenied. - Empty off-Windows, without psutil, or with no other instances. Never raises. + Windows blocks DELETE/REPLACE on a running .exe, so a Desktop-spawned ``hermes.EXE`` makes + the update's quarantine rename fail with ``[WinError 32]``. Excludes our PID and every + *shim* ancestor (the setuptools launcher is a separate native process from the + ``python.exe`` it loads); ``proc.parents()`` at once because a per-hop loop bailed on the + first AccessDenied. Empty off-Windows / without psutil. Never raises. """ if not _m()._is_windows(): return [] @@ -539,10 +521,10 @@ def _process_ppid(pid: int) -> int | None: # --- SSH remote-backend lock ownership ------------------------------------- # ``backend.lock.json`` is written by the Desktop SSH runtime on the *remote* host for every -# ``hermes serve`` it spawns (apps/desktop/electron/remote-lifecycle.ts). A backend another -# client started is legitimate and lock-owned even with no parent here (sshd exited → ppid 1); -# the reap must NEVER kill a PID a valid lock claims — that once killed a production backend. -# Schema constants mirror the writer; a mismatched record is ignored (the reap only *spares*). +# ``hermes serve`` it spawns (apps/desktop/electron/remote-lifecycle.ts). Such a backend is +# legitimate even with no parent here (sshd exited → ppid 1); the reap must NEVER kill a PID a +# valid lock claims — that once killed a production backend. Schema mirrors the writer; a +# mismatched record is ignored (the reap only *spares*). _LOCKFILE_SCHEMA_VERSION = 2 _PROTOCOL_VERSION = 1 _REMOTE_LOCK_SUBDIR = "desktop-ssh" @@ -581,9 +563,8 @@ def _valid_lockfile_payload(parsed: object, ownership_id: str) -> bool: value = parsed.get(field) if not isinstance(value, str) or len(value) > 1024: return False - # logPath is ``{lock_root}/{ownershipId}/{spawnNonce}.log``. Only the suffix is checked so - # a relocated HERMES_HOME doesn't falsely reject a legitimate remote-owned backend (a false - # reject re-introduces the kill). + # logPath is ``{lock_root}/{ownershipId}/{spawnNonce}.log``; only the suffix is checked so a + # relocated HERMES_HOME can't falsely reject a legitimate backend (= re-introduce the kill). return parsed["logPath"].endswith(f"/{ownership_id}/{parsed['spawnNonce']}.log") @@ -636,18 +617,15 @@ def _reap_orphaned_desktop_local_serves( *, reason: str = "orphaned desktop-local hermes serve", signal_term=None, signal_kill=None, sleep_fn=None, lock_owned_pids_fn=None, process_age_seconds_fn=None, ) -> dict[str, list]: - """Kill leftover Desktop-local ``hermes serve`` backends with no parent. + """Kill leftover Desktop-local ``hermes serve`` backends with no parent. Never raises. - When Electron dies uncleanly, ``serve --host 127.0.0.1 --port 0`` children get reparented - to pid 1 with their MCP trees alive; each Desktop boot then stacks a fresh backend on the - corpses until EMFILE. The parent-death watchdog prevents *future* orphans; this clears - *already* orphaned ones. A candidate is reaped only if ALL hold: Desktop-local shape - (never a fixed-port remote serve); ppid 1 (or 0); not self / parent / - HERMES_DESKTOP_CHILD_PID; not claimed by a valid ``backend.lock.json`` (SSH backends other - clients started legitimately sit at ppid 1 — killing them is an incident); older than - ``_REAP_MIN_AGE_SECONDS`` with a determinable age (Desktop writes the lock only after - HERMES_BACKEND_READY, so a live sibling in concurrent multi-profile startup is briefly - unowned and indistinguishable from a corpse). Best-effort; never raises. + When Electron dies uncleanly its ``serve --host 127.0.0.1 --port 0`` children are + reparented to pid 1 with their MCP trees alive; each Desktop boot then stacks a fresh + backend on the corpses until EMFILE. Reaped only if ALL hold: Desktop-local shape; ppid + 0/1; not self / parent / HERMES_DESKTOP_CHILD_PID; not claimed by a valid + ``backend.lock.json`` (SSH backends other clients started legitimately sit at ppid 1); + older than ``_REAP_MIN_AGE_SECONDS`` with a determinable age (Desktop writes the lock only + after HERMES_BACKEND_READY, so a live sibling mid-startup is briefly unowned). """ import signal as _signal import time as _time