diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 27de8d778a..ed58c638b0 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -11,12 +11,12 @@ that target ``hermes_cli.main.`` (``PROJECT_ROOT``, ``_is_windows``, every public-ish name from here (``# noqa: F401``) so the argparse wiring and the test-patch surface still resolve on ``hermes_cli.main``. -Three self-contained closures nested inside ``_cmd_update_impl`` -(``_print_items``, ``_wait_for_service_active``, ``_service_restart_sec``) were -hoisted to module level; they capture no enclosing state (verified via -``symtable``). ``_restart_one_systemd_gateway_unit``, ``_resolve_manage_cmd`` -and ``_on_unit_timeout`` DO capture enclosing locals and stay nested, -byte-identical. +The closures that used to be nested inside ``_cmd_update_impl`` (``_print_items``, +``_wait_for_service_active``, ``_service_restart_sec``, ``_resolve_manage_cmd``, +``_restart_one_systemd_gateway_unit``) now live at module level or inside the +phase helpers ``_cmd_update_impl`` calls in order: ``_pull_updates`` -> +``_sync_python_dependencies_after_pull`` -> ``_run_post_update_maintenance`` -> +``_restart_gateway_fleet_after_update`` -> ``_verify_fleet_after_update``. Imports are one-way: ``hermes_cli.main`` imports this module, never the reverse at import time (``_m()`` resolves lazily at call time, when main.py is fully @@ -32,6 +32,7 @@ import shutil import subprocess import sys import time as _time +from dataclasses import dataclass from datetime import datetime from pathlib import Path from typing import Optional @@ -45,10 +46,8 @@ logger = logging.getLogger(__name__) def _m(): """Lazy ``hermes_cli.main`` reference. - Lets callers keep patching ``hermes_cli.main.`` (the historical - test surface) and have those patches reach this code path, and defers the - import so ``hermes_cli.main`` -> ``hermes_cli.update_cmd`` stays one-way - at import time. + Keeps ``hermes_cli.main.`` test patches effective in this code + path and keeps the ``main`` -> ``update_cmd`` import one-way at import time. """ from hermes_cli import main @@ -106,25 +105,20 @@ _STALE_PURGE_PROTECTED = frozenset( def _purge_stale_hermes_modules() -> None: """Evict every cached Hermes module after the checkout changed in-place. - ``hermes update`` keeps running in the pre-pull Python process. The - gateway auto-restart phase that follows does function-level - ``from hermes_cli.gateway import ...`` — executing NEW source inside an - OLD ``sys.modules`` world. The moment new source references a symbol - that was added to an already-cached module, the import dies (2026-08-20 - field failure: freshly-pulled ``hermes_cli.gateway`` does - ``from hermes_cli.cli_output import line_input``, but ``cli_output`` was - cached from before d0132b582 which introduced ``line_input`` → the whole - restart phase aborted and the gateway kept serving pre-update code). + ``hermes update`` keeps running in the pre-pull Python process; the + gateway-restart phase then does function-level imports of NEW source + inside an OLD ``sys.modules`` world. As soon as new source references a + symbol added to an already-cached module, the import dies (2026-08-20: + fresh ``hermes_cli.gateway`` imported ``line_input`` from a stale cached + ``cli_output`` → restart phase aborted, gateway kept serving old code). - ``_UPDATE_RUNTIME_RELOAD_MODULES`` handled this per-symptom — three - hardcoded module names, re-fixed every time a new module grew a new - export. This is the class fix: drop EVERY cached module under the Hermes - package prefixes so subsequent lazy imports rebuild a self-consistent, - all-new module graph from the updated checkout. Old module objects - referenced by the running updater frames stay alive and functional (a - purge only removes the ``sys.modules`` cache entry); only genuinely - executing modules are exempted, because reloading-in-place — not purging - — is the operation that can pull code out from under a running frame. + ``_UPDATE_RUNTIME_RELOAD_MODULES`` fixed this per-symptom; this is the + class fix: drop EVERY cached module under the Hermes package prefixes so + later lazy imports rebuild a self-consistent graph from the new checkout. + Purging only removes the ``sys.modules`` entry — module objects held by + running frames stay alive and functional. Only genuinely executing modules + are exempt, because reload-in-place (not purge) is what can pull code out + from under a running frame. Best-effort: never raises. """ @@ -156,11 +150,9 @@ def _purge_stale_hermes_modules() -> None: def _reload_updated_runtime_modules() -> None: """Reload update-sensitive modules after the checkout changes in-place. - ``hermes update`` keeps running in the pre-pull Python process. After a - large update, modules already present in ``sys.modules`` can still expose - old symbols even though their source files on disk are new. Refresh the - small module set used by lazy-backend refresh before that step imports - newly-updated code paths. + ``hermes update`` runs in the pre-pull process, so cached modules can + expose old symbols despite new source on disk. Refresh the small set used + by lazy-backend refresh before that step imports newly-updated code paths. """ try: import importlib @@ -181,24 +173,16 @@ def _reload_updated_runtime_modules() -> None: def _reload_config_modules() -> None: """Force-reload modules from disk after git pull. - ``hermes update`` runs in the PRE-pull Python process. After ``git pull`` - updates the source files on disk, modules already in ``sys.modules`` - still hold the OLD code. Function-level imports return the cached module, - so ``DEFAULT_CONFIG["_config_version"]`` is the OLD value and - ``check_config_version()`` reports ``(33, 33)`` — "up to date" — even - though the freshly-pulled code has v34 with a migration to run. + ``hermes update`` runs in the PRE-pull process, so cached modules hold OLD + code: ``DEFAULT_CONFIG["_config_version"]`` is stale and + ``check_config_version()`` reports "up to date" even when the pulled code + has a newer version with a migration to run. Reloads + ``config_defaults`` / ``config`` / ``config_migrations`` from disk. - This function force-reloads ``hermes_cli.config_defaults``, - ``hermes_cli.config``, and ``hermes_cli.config_migrations`` from disk - so subsequent imports read the UPDATED code. - - It also reloads ``hermes_cli._subprocess_compat`` and - ``hermes_cli.dashboard_procs`` so that post-update dashboard cleanup - (``_finish_dashboard_update_cleanup`` → ``_scan_dashboard_processes``) - uses the freshly-pulled code. Without this, a new symbol added to - ``_subprocess_compat`` (e.g. ``bounded_probe_run``) is invisible to the - cached module object, causing ``ImportError`` during the cleanup step - that runs later in the same process. + Also reloads ``_subprocess_compat`` and ``dashboard_procs`` so the later + dashboard cleanup (``_finish_dashboard_update_cleanup`` → + ``_scan_dashboard_processes``) sees symbols the update added (e.g. + ``bounded_probe_run``) instead of dying with ImportError in this process. """ import importlib @@ -311,13 +295,11 @@ def _check_and_apply_config_migration( ) -> None: """Check and apply configuration migrations on an update completion path (#91360). - CRITICAL: ``check_config_version`` and ``migrate_config`` must use - freshly-reloaded modules, not the ``sys.modules`` cache (see - ``_reload_config_modules``). This must run on EVERY update completion - path — the normal post-pull path, the venv-repair retry and the - Node-deps repair on the ``commit_count == 0`` "Already up to date" - branch — so an interrupted update that previously pulled new code does - not strand the user on an older config version. + Must use freshly-reloaded modules (see ``_reload_config_modules``), and + must run on EVERY completion path — normal post-pull, venv-repair retry, + and the Node-deps repair on the ``commit_count == 0`` branch — so an + interrupted update that already pulled new code doesn't strand the user + on an older config version. """ print() print("→ Checking configuration for new options...") @@ -351,11 +333,9 @@ def _check_and_apply_config_migration( needs_migration = has_new_options or current_ver < latest_ver if version_bump_only: - # Nothing for the user to fill in — only the config format version - # changed (new defaults already merge in transparently). Asking - # "configure new options now?" here is misleading: saying yes just - # bumps the version and looks like a no-op (issue: ScottFive / - # Tt2021). Apply it silently and say what actually happened. + # Only the format version changed (new defaults merge transparently). + # Prompting "configure new options now?" would look like a no-op on + # yes (ScottFive / Tt2021) — apply silently and say what happened. print() print( f" ℹ Updating config format (v{current_ver} → v{latest_ver})…" @@ -365,13 +345,10 @@ def _check_and_apply_config_migration( interactive=False, quiet=True ) print(" ✓ Config format updated (no new settings to configure)") - # quiet=True also mutes migration steps that RESET or REMOVE an - # existing setting (e.g. the v33→v34 personality reset from - # #81946, which records its note only in the results dict). - # Re-surface those notes so an unattended update never silently - # changes user configuration (#86656). In this branch - # missing_config is empty, so config_added can only contain - # migration-step mutations, not missing-key listings. + # quiet=True also mutes steps that RESET/REMOVE a setting (e.g. the + # v33→v34 personality reset, #81946). Re-surface them so an + # unattended update never silently changes config (#86656). Here + # missing_config is empty, so config_added holds only mutations. for _note in _mig_results.get("config_added") or []: print(f" ℹ {_note}") for _warn in _mig_results.get("warnings") or []: @@ -383,27 +360,6 @@ def _check_and_apply_config_migration( print() # Show WHAT changed, not just a count, so the user can make an # informed yes/no decision (previously the prompt named nothing). - def _print_items(items, label, key, fallback_key=None): - if not items: - return - print(f" {label}:") - shown = items[:8] - for it in shown: - if isinstance(it, dict): - name = it.get(key) or (fallback_key and it.get(fallback_key)) or "?" - desc = (it.get("description") or "").strip() - else: - # Defensive: some callers/mocks pass bare name strings. - name = str(it) - desc = "" - if desc: - print(f" • {name} — {desc}") - else: - print(f" • {name}") - extra = len(items) - len(shown) - if extra > 0: - print(f" … and {extra} more") - if missing_env: print( f" ⚠️ {len(missing_env)} new required setting(s) need configuration" @@ -440,10 +396,8 @@ def _check_and_apply_config_migration( except EOFError: response = "n" except UnicodeDecodeError: - # input() can raise this when the terminal encoding can't - # decode the byte sequence (e.g. a non-UTF-8 locale, or an - # embedded terminal). Without this, the exception escapes - # here and crashes the update at this prompt. + # Non-UTF-8 locales / embedded terminals can make input() + # raise this; uncaught, it crashes the update at this prompt. print( " ⚠ Could not read input (encoding issue). Skipping. " "Run 'hermes config migrate' manually to configure." @@ -452,11 +406,9 @@ def _check_and_apply_config_migration( if response in {"", "y", "yes", "auto"}: print() - # Gateway mode, --yes, and non-interactive update contexts - # (dashboard / web server actions) cannot prompt for API keys. - # Still run the non-interactive migration pass before restarting - # so new default config fields and version bumps are written - # before the freshly updated gateway validates config at startup. + # Gateway mode, --yes and non-interactive contexts can't prompt + # for API keys; still run the non-interactive pass so new defaults + # and version bumps land before the restarted gateway validates. interactive_migration = not ( gateway_mode or assume_yes or response == "auto" ) @@ -473,15 +425,11 @@ def _check_and_apply_config_migration( else: print(" ✓ Configuration is up to date") - # Fleet-wide config migration (#91277 Phase 2; #20438 earliest report, - # #54926, #79048): the shared checkout serves EVERY profile, but the - # migration above only touched the active profile's config.yaml. - # Sibling profiles kept their old _config_version and silently - # drifted (field repro: sibling gateway restarted onto new code but - # stayed at config v33 vs v37). Run the same NON-INTERACTIVE safe - # migration for every sibling profile home, scoped via the - # context-local HERMES_HOME override (never os.environ — other - # threads must not see it). + # Fleet-wide config migration (#91277 Phase 2; #20438/#54926/#79048): + # the migration above touched only the active profile; siblings drifted + # (field repro: gateway on new code but config v33 vs v37). Run the same + # NON-INTERACTIVE migration per sibling home via the context-local + # HERMES_HOME override (never os.environ — other threads must not see it). try: _migrated_siblings = _migrate_sibling_profile_configs() for _name, _from_ver, _to_ver in _migrated_siblings: @@ -492,12 +440,9 @@ def _check_and_apply_config_migration( except Exception as exc: logger.debug("Sibling config migration failed: %s", exc) - # Safety net: config-version migrations have been observed to leave - # cron/jobs.json valid-but-empty, silently dropping every scheduled - # job (issue #34600). The desktop scheduler can also overwrite with - # its own small set, causing partial loss (issue #52144). If the - # live file now has fewer jobs than the pre-update snapshot, restore - # it and warn loudly. + # Safety net: migrations have left cron/jobs.json valid-but-empty + # (#34600) and the desktop scheduler has overwritten it with a partial + # set (#52144). Restore from the pre-update snapshot if jobs went missing. try: from hermes_cli.backup import restore_cron_jobs_if_emptied @@ -513,11 +458,9 @@ def _check_and_apply_config_migration( # Never let the cron safety net break an otherwise-good update. logger.debug("Cron jobs auto-restore check failed: %s", exc) - # #64160: config.yaml model/provider + MoA safety net. Desktop - # update/repair cycles have rewritten user-set model.provider / - # model.default and dropped the moa: section — settings the gateway - # and cron jobs also consume. Compare the live config against the - # same pre-update snapshot and restore only the protected keys. + # #64160: Desktop update/repair cycles have rewritten model.provider / + # model.default and dropped moa: (settings the gateway and cron consume). + # Restore only those protected keys from the same pre-update snapshot. try: from hermes_cli.backup import restore_config_model_settings_if_rewritten @@ -572,12 +515,9 @@ def _check_and_apply_config_migration( logger.debug("Sibling config auto-restore check failed: %s", exc) -# Critical files that Hermes must be able to import immediately after an -# update/install. Most are imported on every CLI startup; ``web_server.py`` -# is the desktop/dashboard backend path that a fresh Windows install launches -# right away. If any of these fail to parse after a pull, the user can be -# left with a bricked CLI or desktop backend. The post-pull syntax guard -# validates these and auto-rolls-back on failure. +# Files that must parse right after an update/install (CLI startup imports; +# ``web_server.py`` is the desktop backend a fresh Windows install launches). +# The post-pull syntax guard validates these and auto-rolls-back on failure. _UPDATE_CRITICAL_FILES = ( "hermes_cli/main.py", "hermes_cli/config.py", @@ -590,16 +530,36 @@ _UPDATE_CRITICAL_FILES = ( "hermes_constants.py", ) +def _record_update_step(step: str, ok: bool, detail: str = "") -> None: + """Best-effort ``update_receipt.record_step``; the receipt must never break an update.""" + try: + from hermes_cli.update_receipt import record_step + + record_step(step, ok, detail) + except Exception: + pass + + +def _git_run(git_cmd, args, cwd=None, *, check=False, network=False): + """Run ``git_cmd + args`` (default cwd: the checkout), capturing utf-8 text. + + ``network=True`` (fetch/pull/push) disables git's terminal prompt so an + HTTP 401 fails fast instead of hanging on ``Username for ...``. + """ + return subprocess.run( + git_cmd + args, + cwd=_m().PROJECT_ROOT if cwd is None else cwd, + capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=check, + **(_no_prompt_git_kwargs() if network else {}), + ) + + def _capture_head_sha(git_cmd, cwd) -> str | None: """Return the current HEAD SHA, or None if it can't be resolved.""" try: - result = subprocess.run( - git_cmd + ["rev-parse", "HEAD"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - check=True, - ) + result = _git_run(git_cmd, ["rev-parse", "HEAD"], cwd, check=True) return result.stdout.strip() or None except (subprocess.CalledProcessError, OSError): return None @@ -618,36 +578,23 @@ def _prune_orphan_rescue_refs( Each orphan-history divergence (#87694) parks the pre-reset HEAD under ``refs/hermes-update-backups/orphan---``. A rescue ref - pins every object reachable from that commit against ``git gc`` — and in - the incident shape those objects include a full working-tree snapshot - (the autostash orphan commit), which can be multi-GB when the tree holds - large stray files. Left alone, a repeatedly corrupted install would grow - ``.git`` without bound. + pins its objects against ``git gc`` — in the incident shape a full + working-tree snapshot, potentially multi-GB — so a repeatedly corrupted + install would grow ``.git`` without bound. - Two independent limits, both enforced on every orphan incident: - - - **Count cap:** keep only the ``keep`` most-recent refs. - - **Age expiry:** drop any ref older than ``max_age_days``, parsed from - the ``YYYYMMDD-HHMMSS`` timestamp embedded in the ref name (refs with - unparseable names are left alone rather than guessed at). - - Ref names sort chronologically (timestamp prefix), so lexicographic - order from ``for-each-ref`` is also creation order. Deleting a ref makes - its objects eligible for ``git gc``; actual disk reclaim happens on the - next gc (git auto-gc, or the user running ``git gc``). Best-effort: any - failure here must not block the update itself. + Two limits, both enforced on every orphan incident: keep only the + ``keep`` most-recent refs, and drop any older than ``max_age_days`` per + the ``YYYYMMDD-HHMMSS`` stamp in the ref name (unparseable names are left + alone). Names sort chronologically, so ``for-each-ref`` order is creation + order. Disk is reclaimed on the next ``git gc``. Best-effort: never + blocks the update. """ try: - list_result = subprocess.run( - git_cmd + [ - "for-each-ref", - "--format=%(refname)", - "--sort=refname", - f"refs/hermes-update-backups/orphan-{branch}-*", - ], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", + list_result = _git_run( + git_cmd, + ["for-each-ref", "--format=%(refname)", "--sort=refname", + f"refs/hermes-update-backups/orphan-{branch}-*"], + cwd, ) if list_result.returncode != 0: return @@ -671,12 +618,7 @@ def _prune_orphan_rescue_refs( if ref_time < cutoff: stale.add(ref) for ref in sorted(stale): - subprocess.run( - git_cmd + ["update-ref", "-d", ref], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + _git_run(git_cmd, ["update-ref", "-d", ref], cwd) except OSError: pass @@ -693,21 +635,17 @@ _INSTALL_DEFINING_FILES = ( def _editable_install_is_current(git_cmd, cwd, pre_pull_sha: str | None) -> bool: """True when the pulled commits cannot have invalidated the editable install. - ``uv pip install -e .`` never audits an editable target — it reinstalls on - every invocation, and every reinstall rewrites the console-script shims. - On Windows that rewrite is the only reason the running ``hermes.exe`` has - to be quarantined, and a quarantine that loses its race is the whole - ``os error 32`` family. Not reinstalling when the reinstall provably - cannot change anything removes that risk outright for the common update, - rather than trying to make the rename win more often. + ``uv pip install -e .`` reinstalls unconditionally and rewrites the + console-script shims every time. On Windows that rewrite is the only + reason the running ``hermes.exe`` must be quarantined, and a lost + quarantine race is the whole ``os error 32`` family — so skip the + reinstall when it provably cannot change anything. - Skipping is safe because Hermes pins its editable finder to a *static* - module list (``[tool.setuptools] py-modules`` plus - ``packages.find.include``). The one source-only change that would stale - that finder is a new top-level module or package, and it cannot land - without a ``pyproject.toml`` diff. Dependencies and ``[project.scripts]`` - live there too. New submodules inside an already-mapped package resolve - through the real package directory and need no reinstall. + Safe because Hermes pins its editable finder to a *static* module list + (``[tool.setuptools] py-modules`` + ``packages.find.include``): only a + new top-level module/package can stale it, and that needs a + ``pyproject.toml`` diff (as do dependencies and ``[project.scripts]``). + New submodules under an already-mapped package need no reinstall. Fails closed: an unresolvable pre-pull SHA (shallow checkout, ZIP swap) or a failed ``git diff`` returns False and the install runs as before. @@ -755,21 +693,16 @@ def _validate_python_files_syntax( def _validate_critical_files_syntax(root) -> tuple[bool, str | None, str | None]: """Compile each file in ``_UPDATE_CRITICAL_FILES`` to catch SyntaxErrors. - These are the files imported on every ``hermes`` startup; if any of them - has a syntax error (orphan merge-conflict markers, bad ref to a name - that no longer exists, etc.) the CLI can't bootstrap at all. We validate - them after a successful ``git pull`` so we can auto-roll-back instead of - leaving the user with a bricked install. + These are imported on every ``hermes`` startup; a syntax error (orphan + conflict markers, etc.) means the CLI can't bootstrap, so we validate + after ``git pull`` and auto-roll-back instead of leaving a bricked install. - The compiled ``.pyc`` is written to a temp directory rather than the - source tree's ``__pycache__/`` so we don't race with concurrent test - workers that walk the same dir, and so we don't leave a stale pyc - behind in production if the next interpreter run picks a different - Python version. The pyc is discarded on function return either way — - we only care about the compile-or-not signal. + The ``.pyc`` goes to a temp dir, not the tree's ``__pycache__/``: avoids + racing concurrent test workers and leaving a stale pyc behind when the + next interpreter run uses a different Python. Only the compile-or-not + signal matters. - Returns ``(ok, failing_path, error_message)``. ``ok=True`` means every - file parsed cleanly. + Returns ``(ok, failing_path, error_message)``. """ return _validate_python_files_syntax(root, _UPDATE_CRITICAL_FILES) @@ -791,31 +724,21 @@ def _critical_module_import_failures( ) -> dict[str, tuple[str, str]]: """Import each module in ``_UPDATE_CRITICAL_MODULES`` in a subprocess. - ``_validate_critical_files_syntax`` only *parses* files, so it cannot see - cross-module breakage: a partially-updated tree where ``agent/`` is new but - ``tools/`` is old parses perfectly and still dies at startup with - ``ImportError: cannot import name 'TODO_INJECTION_HEADER' from - 'tools.todo_tool'``. Every file is valid Python; the *combination* is not. + ``_validate_critical_files_syntax`` only *parses*, so a partially-updated + tree (new ``agent/``, old ``tools/``) parses fine yet dies at startup + with ``ImportError: cannot import name ...``. That skew is reachable on + the Windows ZIP-update path, whose copy loop replaces top-level entries + one at a time in ``os.listdir`` order. - That skew is reachable on the Windows ZIP-update path, whose copy loop - walks top-level entries in ``os.listdir`` order and replaces each one - independently — ``agent/`` lands long before ``tools/``, so a failure or - interruption between them leaves exactly that mismatch on disk. + Runs in a subprocess (~0.4s) so the half-updated tree's import-time side + effects don't pollute the updater's ``sys.modules``. Uses the project + venv's interpreter when present (like ``_venv_core_imports_healthy``): + ``hermes update`` may be driven by a different Python than the install's. - Runs in a subprocess because importing these modules into the running - updater would pollute ``sys.modules`` and execute import-time side effects - against the half-updated tree. Costs ~0.4s. - - Uses the project venv's interpreter when there is one (matching - ``_venv_core_imports_healthy``): ``hermes update`` can be driven by a - different Python than the install's own, and probing the wrong - interpreter would test a tree the user never runs. - - Returns every failing module in probe order. Generic import-time exceptions - remain tolerated by default because they can depend on local config or - environment. ``report_runtime_errors=True`` exposes them so a caller can - compare two states of the same checkout without an earlier failure masking - a later one. + Returns every failing module in probe order. Generic import-time + exceptions are tolerated by default (they can depend on local config); + ``report_runtime_errors=True`` exposes them so a caller can compare two + states of the same checkout without one failure masking another. """ from hermes_constants import FIRST_PARTY_MODULE_ROOTS @@ -929,12 +852,10 @@ def _validate_critical_modules_import( def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) -> str: """File-based IPC prompt for gateway mode. - Writes a prompt marker file so the gateway can forward the question to the - user, then polls for a response file. Falls back to *default* on timeout. - - Used by ``hermes update --gateway`` so interactive prompts (stash restore, - config migration) are forwarded to the messenger instead of being silently - skipped. + Writes a prompt marker file for the gateway to forward to the user, then + polls for a response file; falls back to *default* on timeout. Lets + ``hermes update --gateway`` forward prompts (stash restore, config + migration) to the messenger instead of silently skipping them. """ import json as _json import uuid as _uuid @@ -944,7 +865,6 @@ def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) prompt_path = home / ".update_prompt.json" response_path = home / ".update_response" - # Clean any stale response file response_path.unlink(missing_ok=True) payload = { @@ -956,7 +876,6 @@ def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) tmp.write_text(_json.dumps(payload), encoding="utf-8") tmp.replace(prompt_path) - # Poll for response deadline = _time.monotonic() + timeout while _time.monotonic() < deadline: if response_path.exists(): @@ -969,7 +888,6 @@ def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) pass _time.sleep(0.5) - # Timeout — clean up and use default prompt_path.unlink(missing_ok=True) response_path.unlink(missing_ok=True) print(f" (no response after {int(timeout)}s, using default: {default!r})") @@ -1163,16 +1081,11 @@ def _print_fts_optimize_available_notice() -> None: def _print_curator_recent_run_notice() -> None: """Print the most recent curator run summary, exactly once. - The curator runs in the background (gateway tick + CLI session start), - so users learn about skill consolidations only by stumbling into a - rename. ``hermes update`` is a high-attention surface — surface the - most recent run's rename map here, once. - - Show-once: state stamps ``last_run_summary_shown_at`` after printing. - Subsequent ``hermes update`` invocations skip the block until a newer - curator run lands. Silent when the curator has never run, when the - most recent summary has already been shown, or when the summary has - no rename information to display (no archives). + The curator runs in the background, so users only notice consolidations + by stumbling into a rename; ``hermes update`` is a high-attention surface + to show the rename map. Show-once: stamps ``last_run_summary_shown_at`` + after printing. Silent when the curator never ran, the summary was already + shown, or it has no rename info (no archives). """ try: from agent import curator @@ -1194,10 +1107,8 @@ def _print_curator_recent_run_notice() -> None: if not summary: return - # Only print when there's something interesting to show — i.e. the - # rename map block was appended (multi-line summary). A bare "auto: - # no changes; llm: no change" doesn't warrant interrupting the - # update flow. + # Only a multi-line summary (rename map appended) is worth showing; a + # bare "auto: no changes; llm: no change" isn't. if "\n" not in summary: # Still stamp it shown so we don't reconsider it on every update. try: @@ -1207,7 +1118,6 @@ def _print_curator_recent_run_notice() -> None: pass return - # Format the timestamp as "Xh ago" for readability. when = _format_time_ago(last_run_at) print() print(f"ℹ Skill curator — last run {when}") @@ -1247,18 +1157,15 @@ def _format_time_ago(iso_ts: str) -> str: def _reload_process_scan_modules() -> None: """Force-reload the process-scan modules from disk after an update. - ``_finish_dashboard_update_cleanup`` runs in the PRE-update Python - process, but ``_scan_dashboard_processes`` does a function-level - ``from hermes_cli._subprocess_compat import bounded_probe_run``. If the - update added a new symbol to ``_subprocess_compat`` (as #87134 did with - ``bounded_probe_run``), the cached OLD module object doesn't have it and - the cleanup step crashes with ImportError — after the code update itself - already succeeded. Reload dependency-first so ``dashboard_procs`` binds - against the fresh ``_subprocess_compat``. + ``_finish_dashboard_update_cleanup`` runs in the PRE-update process, but + ``_scan_dashboard_processes`` lazily imports from ``_subprocess_compat``; + a symbol the update added (``bounded_probe_run``, #87134) is missing from + the cached OLD module and the cleanup crashes with ImportError after the + code update already succeeded. Reload dependency-first so + ``dashboard_procs`` binds against the fresh ``_subprocess_compat``. - Lives here (called from the cleanup entry point) rather than only in - ``_reload_config_modules`` so EVERY caller — the git-update path, the - Windows ZIP fallback path, and any future one — is covered. + Called from the cleanup entry point (not only ``_reload_config_modules``) + so EVERY caller — git path, Windows ZIP fallback, future ones — is covered. """ import importlib @@ -1318,16 +1225,13 @@ def _finish_dashboard_update_cleanup( def _atomic_replace_dir(src: str, dst: str) -> None: """Replace directory *dst* with *src* without leaving *dst* half-deleted. - The naive ``rmtree(dst); copytree(src, dst)`` has a destructive window: if - the copy fails partway (common on the Windows ZIP-update path, which only - runs because file I/O is already flaky on that machine), the old directory - is already gone and nothing replaced it — the install is left with a - deleted tree (issue #49145, where ``ui-tui/`` vanished and broke the TUI). + Naive ``rmtree(dst); copytree(src, dst)`` has a destructive window: a + copy that fails partway (common on the Windows ZIP path, which only runs + because file I/O is already flaky) leaves the old tree gone and nothing + in its place (#49145: ``ui-tui/`` vanished and broke the TUI). - Now a thin single-entry alias over the two-phase helpers below, which - generalise the same stage-then-swap discipline across every entry the ZIP - update touches (#76104). Retained because it is part of the mechanical - ``hermes_cli.main`` re-export surface and guards the #49145 regression. + Now a thin alias over the two-phase helpers below (#76104); retained as + part of the ``hermes_cli.main`` re-export surface and the #49145 guard. """ _commit_staged_replacements([(_stage_replacement(src, dst), dst)]) @@ -1342,11 +1246,9 @@ def _stage_replacement(src: str, dst: str) -> str: staging = f"{dst}.hermes-update-staging" backup = f"{dst}.hermes-update-old" # A previous run may have died between "move dst aside" and "move staging - # in" — leaving dst missing and the backup as the ONLY copy of that entry. - # Restore it before clearing leftovers: deleting the backup first and then - # failing to stage (disk exhaustion is likely right after writing a full - # staging copy) would leave a hole in the install with nothing to roll - # back to. The restore is a same-filesystem rename — instant and safe. + # in", leaving the backup as the ONLY copy. Restore it BEFORE clearing + # leftovers: deleting it and then failing to stage (disk exhaustion is + # likely here) would leave a hole with nothing to roll back to. if not os.path.exists(dst) and os.path.exists(backup): os.rename(backup, dst) for leftover in (staging, backup): @@ -1364,11 +1266,9 @@ def _stage_replacement(src: str, dst: str) -> str: def _discard_staged(staged) -> None: """Remove staging paths for entries that were never committed. - Without this a phase-1 failure (typically disk exhaustion) orphans one - staging copy per entry already processed — up to a full second copy of - the tree. The user then follows the "re-run `hermes update`" advice with - *less* free space than before and the retry fails harder than the - original attempt. + Otherwise a phase-1 failure (typically disk exhaustion) orphans one + staging copy per processed entry — up to a full second tree — and the + advised "re-run `hermes update`" retry fails harder with less free space. """ for staging, _dst in staged: try: @@ -1383,25 +1283,20 @@ def _discard_staged(staged) -> None: def _commit_staged_replacements(staged) -> None: """Phase 2: swap every staged entry into place, rolling back all on failure. - ``_atomic_replace_dir`` makes each *individual* directory swap safe, but - the ZIP update replaces ~90 top-level entries in a loop, and nothing made - the loop atomic *as a whole*. A failure partway left some entries at the - new version and the rest at the old one — every file valid Python, the - combination unbootable (issue #76104; the ``ImportError`` in #76091 and - the field report in #63717 are both this). + ``_atomic_replace_dir`` made each *individual* swap safe, but the ZIP + update loops over ~90 top-level entries and nothing made the loop atomic + *as a whole*: a partway failure left a mixed-version tree — every file + valid, the combination unbootable (#76104; also #76091, #63717). - This covers plain files as well as directories: the repo root holds 20 - first-party modules (``run_agent.py``, ``cli.py``, ``hermes_constants.py`` - …), so a files-only failure reproduces exactly the bug class we are - closing. Every swap is an ``os.rename`` onto a path that was just moved - aside — a same-filesystem rename is atomic on POSIX and NTFS alike, so a - file swap can never leave a half-written module the way ``copy2`` onto a - live path can. + Covers plain files too: the repo root holds 20 first-party modules, so a + files-only failure reproduces the same bug class. Every swap is an + ``os.rename`` onto a just-moved-aside path — atomic on POSIX and NTFS — + so a file swap can't leave a half-written module the way ``copy2`` onto + a live path can. - Splitting stage-all-then-swap-all shrinks the failure window from "the - duration of a full tree copy" to "the duration of N renames", and makes - the remaining window recoverable: if a swap fails we restore every entry - already swapped, so the tree lands wholly new or wholly old. + Stage-all-then-swap-all shrinks the failure window from "a full tree + copy" to "N renames" and makes it recoverable: a failed swap restores + every entry already swapped, so the tree lands wholly new or wholly old. """ swapped: list[tuple[str, str]] = [] # (dst, backup) in swap order; "" = absent try: @@ -1443,11 +1338,9 @@ def _commit_staged_replacements(staged) -> None: def _branch_head_label(git_cmd=None, cwd=None) -> str | None: """``" @ "`` for the checkout, or None when unknown. - Appended to the update summary lines so branch drift is visible at a - glance (live incident 2026-08-17: a checkout parked on a stale feature - branch got "✓ Update complete!" with nothing on the line saying WHERE - the checkout actually sat). Never raises — summary decoration must not - break an update. + Appended to update summary lines so branch drift is visible (2026-08-17 + incident: a checkout parked on a stale feature branch got "✓ Update + complete!" with nothing saying WHERE it sat). Never raises. """ try: cmd = list(git_cmd) if git_cmd else ["git"] @@ -1486,26 +1379,22 @@ def _assess_parked_branch_switch( """Decide whether it is safe to auto-switch a parked feature branch back to the update target. - Live incident (2026-08-17, Teknium's box): the source checkout sat on a - stale feature branch left behind by earlier tooling; ``hermes update`` - autostashed, ran its post-update steps and printed "✓ Code updated!" - while the running code stayed days behind main. The guard's contract: + Live incident (2026-08-17): the checkout sat on a stale feature branch; + ``hermes update`` autostashed, ran post-update steps and printed + "✓ Code updated!" while the running code stayed days behind main. - - (True, "") when the working tree + index are clean AND every commit on - the parked branch is already contained in ``origin/`` - (``git cherry`` reports no ``+`` lines). - - (True, "unmerged:") when the tree is clean but the branch has - commits not yet in the target. Switching is safe — ``git checkout`` - never discards committed work and the branch keeps the commits — but - the caller must print a LOUD notice naming the branch and count so the - work is not forgotten. This is what non-interactive callers (desktop - update button, gateway /update, cron) rely on: they have no way to - resolve a skip, so a clean checkout must always reach the target. + - (True, "") — tree + index clean AND every parked commit is already in + ``origin/`` (``git cherry`` reports no ``+`` lines). + - (True, "unmerged:") — tree clean but the branch has commits not + in the target. Switching is safe (``git checkout`` never discards + committed work) but the caller must print a LOUD notice naming the + branch and count. Non-interactive callers (desktop button, gateway + /update, cron) rely on this: they can't resolve a skip, so a clean + checkout must always reach the target. - (False, ) — dirty tree, git errors, or the - ``updates.auto_switch_parked_branch: false`` config opt-out — and the - caller must NOT touch the branch. A dirty tree is the one genuinely - unsafe case: uncommitted work would have to ride an autostash across - branches, which is how the 2026-08-17 incident started. + ``updates.auto_switch_parked_branch: false`` opt-out; caller must NOT + touch the branch. A dirty tree is the genuinely unsafe case: uncommitted + work riding an autostash across branches is how the incident started. Block reasons: "disabled", "dirty", "unverifiable". """ @@ -1522,21 +1411,13 @@ def _assess_parked_branch_switch( # fall through to them with the default (auto-switch allowed). logger.debug("Could not read updates.auto_switch_parked_branch: %s", exc) - status = subprocess.run( - git_cmd + ["status", "--porcelain"], - cwd=cwd, capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + status = _git_run(git_cmd, ["status", "--porcelain"], cwd) if status.returncode != 0: return False, "unverifiable" if status.stdout.strip(): return False, "dirty" - cherry = subprocess.run( - git_cmd + ["cherry", f"origin/{target_branch}"], - cwd=cwd, capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + cherry = _git_run(git_cmd, ["cherry", f"origin/{target_branch}"], cwd) if cherry.returncode != 0: return False, "unverifiable" unmerged = [ @@ -1561,11 +1442,7 @@ def _print_parked_branch_skip_warning( branch, with the behind-count and the exact commands to resolve.""" behind = None try: - behind_result = subprocess.run( - git_cmd + ["rev-list", f"HEAD..origin/{target_branch}", "--count"], - cwd=cwd, capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + behind_result = _git_run(git_cmd, ["rev-list", f"HEAD..origin/{target_branch}", "--count"], cwd) if behind_result.returncode == 0 and behind_result.stdout.strip(): behind = int(behind_result.stdout.strip()) except Exception: @@ -1608,11 +1485,9 @@ def _print_parked_branch_kept_notice( """LOUD notice printed when a clean parked branch with unmerged commits is auto-switched back to the update target. - Non-interactive callers (desktop update button, gateway /update, cron) - cannot resolve a skip, so a clean checkout always proceeds to the - target — but the unmerged work must be impossible to miss. The commits - are untouched: ``git checkout`` never discards committed work; the - branch keeps them until the user returns. + Non-interactive callers can't resolve a skip, so a clean checkout always + proceeds — but the unmerged work must be impossible to miss. The commits + stay on the branch (``git checkout`` never discards committed work). """ bar = "=" * 68 print() @@ -1632,13 +1507,10 @@ def _print_parked_branch_kept_notice( def _print_update_completion(message: str) -> None: - """Print an update outcome plus, when the dashboard launched this run - with an action id, a terminal receipt line the Desktop can match after - the dashboard restarts (see #47359 / #58764). - - The outcome line carries the checkout's actual branch + HEAD short-sha - so branch drift is visible at a glance (2026-08-17 parked-branch - incident).""" + """Print an update outcome plus, when the dashboard launched this run with + an action id, a terminal receipt line the Desktop can match after the + dashboard restarts (#47359 / #58764). The outcome line carries the + branch + HEAD short-sha so branch drift is visible (2026-08-17 incident).""" print(f"{message}{_branch_head_suffix()}") action_id = os.environ.get("HERMES_ACTION_ID", "") if len(action_id) == 32 and all(char in "0123456789abcdef" for char in action_id): @@ -1781,15 +1653,13 @@ def _zip_overlay_block_reason( ) -> Optional[str]: """Why overlaying a ZIP onto ``root`` would destroy work, or None if safe. - The ZIP path swaps every top-level entry (except a tiny preserve set) and - then deletes the backups, so uncommitted edits and untracked files under - a replaced directory are gone. Fail closed when git status cannot run: - unknown dirtiness is not a license to clobber the tree (#87304). + The ZIP path swaps every top-level entry (minus a tiny preserve set) and + deletes the backups, so uncommitted edits and untracked files are gone. + Fails closed when git status cannot run (#87304). - ``ignore_staging_artifacts`` is for the pre-swap re-check: phase 1 of the - two-phase replace creates ``*.hermes-update-staging`` siblings inside the - checkout, which git reports as untracked. Those are our own artifacts, - not user work — without the filter the re-check would always refuse. + ``ignore_staging_artifacts`` is for the pre-swap re-check: phase 1 leaves + ``*.hermes-update-staging`` siblings that git reports as untracked; they + are our own artifacts, and without the filter the re-check always refuses. """ if not (root / ".git").exists(): return None @@ -1797,15 +1667,12 @@ def _zip_overlay_block_reason( if sys.platform == "win32": git_cmd = ["git", "-c", "windows.appendAtomically=false"] result = subprocess.run( - # -uall: a user-level ``status.showUntrackedFiles = no`` git config - # would otherwise hide untracked files and silently blind this guard. - # --ignored=matching: gitignored files are still USER DATA the ZIP - # overlay would permanently delete (logs, scratch files, local data) - # — a .gitignore entry must not blind the guard either (#87392). - # ``matching`` reports an ignored directory as one ``dir/`` line - # instead of enumerating its contents (cheaper, same verdict for the - # top-level filter below). NOTE: ``--ignored=all`` is NOT a valid - # git mode — it exits 128 and would fail-close every ZIP update. + # -uall: a user-level ``status.showUntrackedFiles = no`` must not + # blind this guard. --ignored=matching: gitignored files are still + # USER DATA the overlay would delete (#87392); ``matching`` reports an + # ignored dir as one ``dir/`` line (cheaper, same verdict below). + # NOTE: ``--ignored=all`` is NOT a valid git mode — exits 128 and + # would fail-close every ZIP update. git_cmd + ["status", "--porcelain", "--untracked-files=all", "--ignored=matching"], cwd=root, capture_output=True, @@ -1842,13 +1709,11 @@ def _is_zip_preserved_entry_status_line(line: str) -> bool: """True when every path on a porcelain status line sits under a top-level entry the ZIP swap preserves. - The ``" -> "`` two-path split applies ONLY to rename/copy status codes - (R/C): porcelain v1 does not quote a plain filename containing spaces, - so an ignored file literally named ``venv -> node_modules`` on an - ``!!``/``??`` line must be treated as ONE path — splitting it would - filter it as two preserved tops and fail-open into the destructive swap. - Requiring EVERY path preserved keeps renames leaving a preserved dir - (``R venv/x -> src/x``) blocking, fail-closed. + The ``" -> "`` split applies ONLY to rename/copy codes (R/C): porcelain + v1 doesn't quote plain filenames with spaces, so an ignored file named + ``venv -> node_modules`` on a ``!!``/``??`` line is ONE path — splitting + would fail-open into the destructive swap. Requiring EVERY path preserved + keeps renames out of a preserved dir (``R venv/x -> src/x``) blocking. """ status, payload = (line[:2], line[3:]) if len(line) >= 3 else ("", line) is_rename = any(code in "RC" for code in status) @@ -1889,11 +1754,9 @@ def _abort_zip_update_if_dirty_tree() -> None: def _read_project_version() -> str | None: """Read the ``version`` field from the checkout's pyproject.toml. - Reads the on-disk file (not importlib.metadata) because after a git - pull the installed distribution metadata still describes the OLD - version; the file is the only source that reflects what was just - pulled. Returns None on any failure — version reporting is cosmetic - and must never break an update. + On-disk file, not importlib.metadata: after a pull the installed + metadata still describes the OLD version. Returns None on any failure — + version reporting is cosmetic and must never break an update. """ try: import tomllib @@ -1908,11 +1771,9 @@ def _read_project_version() -> str | None: def _update_complete_message(pre_version: str | None) -> str: """Completion line with the version transition when it is known. - Ported from PrimeIntellect-ai/prime-agent#630: after a successful - self-update, show both versions (``v0.19.4 → v0.20.0``) so the user - can see what they actually got. Falls back to the plain message when - either side is unknown or the version did not change (e.g. several - commits landed within one release). + Ported from PrimeIntellect-ai/prime-agent#630: show ``v0.19.4 → v0.20.0`` + after a self-update. Plain message when either side is unknown or the + version did not change (several commits within one release). """ post_version = _read_project_version() if pre_version and post_version and pre_version != post_version: @@ -1970,19 +1831,17 @@ def _print_verified_update_completion(message: str) -> bool: def _clear_stale_sqlite_sidecars(db_path: Path) -> None: """Delete the WAL / shared-memory / rollback-journal files next to *db_path*. - Call this immediately before overwriting a database file with a snapshot - image. Quick snapshots are produced by ``backup._safe_copy_db`` through - ``sqlite3.backup()``, so the image is already checkpointed and owns no WAL — - which is exactly why ``backup._EXCLUDED_SUFFIXES`` refuses to ship sidecars - inside a snapshot. Copying the image over the destination replaces only the - main database file, so any ``-wal`` / ``-shm`` left behind by the *old* - database (a crashed writer, or a second Hermes process the updater's drain - did not stop) survives and is replayed over the fresh image on the next - open. The result passes ``PRAGMA integrity_check`` while serving the old - database's contents, and the first checkpoint folds it in permanently. + Call immediately before overwriting a database with a snapshot image. + Quick snapshots come from ``sqlite3.backup()`` (``backup._safe_copy_db``), + so the image is checkpointed and owns no WAL — which is why + ``backup._EXCLUDED_SUFFIXES`` ships no sidecars. Copying the image + replaces only the main file, so a ``-wal``/``-shm`` left by the *old* + database (crashed writer, undrained second process) is replayed over the + fresh image on next open: it passes ``PRAGMA integrity_check`` while + serving the old contents, and the first checkpoint makes that permanent. - Removing them is safe here specifically: they belong to a database the - caller has already declared corrupt and is about to discard. + Safe here because the sidecars belong to a database the caller has already + declared corrupt and is about to discard. """ for suffix in ("-wal", "-shm", "-journal"): db_path.with_name(db_path.name + suffix).unlink(missing_ok=True) @@ -2046,22 +1905,19 @@ def _write_gateway_update_exit_code(ok: bool) -> None: def _restore_state_db_from_snapshot(state_path: Path, snap_state: Path) -> bool: """Replace *state_path* with the snapshot image at *snap_state*. - Shared by both post-update auto-restore paths (the ZIP update and the git - pull). The destination's stale sidecars are cleared before the copy, so the - restored image cannot be silently overwritten by the corrupt database's WAL - replay — see :func:`_clear_stale_sqlite_sidecars`. + Shared by the ZIP and git-pull auto-restore paths. Stale sidecars are + cleared before the copy so the corrupt database's WAL replay cannot + silently overwrite the restored image (:func:`_clear_stale_sqlite_sidecars`). - Refuses (returns ``False``) while another process — or a live connection - in THIS process — still holds the database or its sidecars open: copying a - snapshot over a live writer's inode makes the writer's page cache and WAL - index disagree with the file bytes, and its next checkpoint writes pages - at offsets that no longer mean what it thinks — the #90950 page-1 clobber. - ``None`` (scan unavailable) proceeds: the updater has already drained - gateways, and refusing on "unknown" would disable auto-restore on every - non-Linux host. + Refuses (``False``) while another process — or a live connection in THIS + process — holds the database or its sidecars: copying over a live + writer's inode desyncs its page cache/WAL index from the file bytes and + its next checkpoint clobbers pages (#90950 page-1 clobber). ``None`` + (scan unavailable) proceeds: gateways are already drained, and refusing + on "unknown" would disable auto-restore on every non-Linux host. Returns ``True`` when the restored file passes an integrity check. Raises - ``OSError`` if the copy itself fails, which callers already report. + ``OSError`` if the copy itself fails (callers already report it). """ from hermes_cli.backup import _foreign_db_holder_pids, verify_sqlite_integrity from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access @@ -2074,15 +1930,12 @@ def _restore_state_db_from_snapshot(state_path: Path, snap_state: Path) -> bool: "then restore manually with /snapshot restore." ) return False - # The foreign-pid scan excludes THIS process on purpose, but an - # in-process SessionDB handle is exactly as much of a live holder: - # unlinking the -wal/-shm and copy2-ing over the main file under it - # leaves this process checkpointing through deleted-inode sidecars — - # the #90950 split brain produced first-party (proven live on main: - # `/proc/self/fd` shows `state.db-wal (deleted)` right after this ran - # under a tracked connection). ``offline_file_access`` fails CLOSED on - # any tracked live connection and holds the connection-lifecycle lock - # across the sidecar clear + copy so none can appear mid-swap. + # The foreign-pid scan excludes THIS process, but an in-process SessionDB + # handle is just as live: unlinking -wal/-shm and copy2-ing under it + # leaves this process checkpointing through deleted-inode sidecars (the + # #90950 split brain, reproduced live via `/proc/self/fd`). + # ``offline_file_access`` fails CLOSED on any tracked connection and holds + # the lifecycle lock across clear + copy so none can appear mid-swap. try: with offline_file_access(state_path, what="restore a snapshot over"): _clear_stale_sqlite_sidecars(state_path) @@ -2166,11 +2019,9 @@ def _verify_and_restore_state_dbs_post_update() -> None: """Post-update integrity guard for the ROOT state.db AND every sibling profile's state.db (#97994). - The pre-update snapshot already covers every sibling profile - (#66140 create_pre_update_snapshots_all_profiles), but the post-update - guard only ever verified the root DB — a profile database corrupted by - the update was never detected and never auto-restored, leaving that - profile's sessions silently gone while the root DB passed. + The pre-update snapshot already covers siblings (#66140), but the guard + only verified the root DB — a corrupted profile DB was never detected or + restored, its sessions silently gone while the root passed. """ home = get_hermes_home() _verify_and_restore_one_state_db(home, label="default home") @@ -2183,6 +2034,52 @@ def _verify_and_restore_state_dbs_post_update() -> None: logger.debug("Sibling-profile state.db guard sweep failed: %s", exc) +def _ensure_venv_pip(pip_cmd: list, python_exe: str) -> None: + """Bootstrap pip back into the venv via ensurepip when ``pip --version`` fails + (some environments lose it); call before the editable install.""" + try: + subprocess.run( + pip_cmd + ["--version"], + cwd=_m().PROJECT_ROOT, + check=True, + capture_output=True, + ) + except subprocess.CalledProcessError: + subprocess.run( + [python_exe, "-m", "ensurepip", "--upgrade", "--default-pip"], + cwd=_m().PROJECT_ROOT, + check=True, + ) + + +def _print_bundled_skills_sync_report() -> None: + """Run ``sync_skills`` (copies new, updates changed, respects user deletions) and print its summary.""" + from tools.skills_sync import sync_skills + + result = sync_skills(quiet=True) + if result["copied"]: + print(f" + {len(result['copied'])} new: {', '.join(result['copied'])}") + if result.get("updated"): + print( + f" ↑ {len(result['updated'])} updated: {', '.join(result['updated'])}" + ) + if result.get("user_modified"): + print(f" ~ {len(result['user_modified'])} user-modified (kept)") + print( + " → see them: hermes skills list-modified " + "(diff/reset to resume updates)" + ) + if result.get("cleaned"): + print(f" − {len(result['cleaned'])} removed from manifest") + if result.get("relocated"): + print( + f" → {len(result['relocated'])} moved to new upstream paths: " + f"{', '.join(result['relocated'])}" + ) + if not result["copied"] and not result.get("updated"): + print(" ✓ Skills are up to date") + + def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> bool: """Update Hermes Agent by downloading a ZIP archive. @@ -2201,12 +2098,9 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo # completion line can report the transition (prime-agent#630 port). pre_update_version = _read_project_version() - # The ZIP fallback exists for Windows git-file-I/O breakage. It pulls a - # static archive from GitHub, which is fine for the default "main" - # channel but would silently ignore --branch and update from main even - # if the user asked for something else — exactly the silent-divergence - # bug --branch was added to prevent. Refuse to proceed in that case - # rather than lie. + # The static GitHub archive is fine for "main" but would silently ignore + # --branch — the exact silent-divergence bug --branch was added to + # prevent. Refuse rather than lie. branch = _m()._resolve_update_branch(args) if branch != "main": print( @@ -2234,11 +2128,9 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo print("→ Extracting...") import stat as _stat with zipfile.ZipFile(zip_path, "r") as zf: - # Validate paths to prevent zip-slip (path traversal) AND reject - # symlink members. A GitHub source ZIP for hermes-agent itself - # should never contain symlinks — they'd point outside the - # extracted tree and let an attacker who can compromise the - # update mirror plant arbitrary files via the update path. + # Reject zip-slip (path traversal) AND symlink members: a + # hermes-agent source ZIP never legitimately contains symlinks, + # and a compromised mirror could use them to plant files anywhere. tmp_dir_real = os.path.realpath(tmp_dir) for member in zf.infolist(): member_path = os.path.realpath(os.path.join(tmp_dir, member.filename)) @@ -2261,29 +2153,21 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo # GitHub ZIPs extract to hermes-agent-/ extracted = os.path.join(tmp_dir, f"hermes-agent-{branch}") if not os.path.isdir(extracted): - # Try to find it for d in os.listdir(tmp_dir): candidate = os.path.join(tmp_dir, d) if os.path.isdir(candidate) and d != "__MACOSX": extracted = candidate break - # Copy updated files over existing installation, preserving venv/node_modules/.git preserve = _ZIP_PRESERVED_TOP_LEVEL entries = [i for i in os.listdir(extracted) if i not in preserve] - # Two-phase replace (#76104). Phase 1 copies every entry — directories - # AND top-level files — to a sibling staging path without touching - # anything live; phase 2 swaps them all in with same-filesystem - # renames and rolls back every swap if any one fails. Replacing - # entries one-at-a-time (the previous shape) meant an interruption - # partway left `agent/` new and `tools/` stale — all files valid, the - # tree unbootable. Files matter as much as directories here: the repo - # root holds 20 first-party modules (run_agent.py, cli.py, - # hermes_constants.py, ...). - # - # Staging costs one extra copy of the tree on disk. Check up front so - # we fail with a clear message instead of running out mid-copy. + # Two-phase replace (#76104): phase 1 stages every entry (dirs AND + # top-level files — the repo root holds 20 first-party modules) beside + # its target; phase 2 swaps all in with same-filesystem renames and + # rolls back on any failure. One-at-a-time replacement left `agent/` + # new and `tools/` stale on interruption: all files valid, tree + # unbootable. Staging costs one extra tree copy — check space up front. need = sum( os.path.getsize(os.path.join(dirpath, f)) for entry in entries @@ -2294,11 +2178,9 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo for e in entries if os.path.isfile(os.path.join(extracted, e)) ) - # Only the staging copy is new — the live tree already occupies its - # space and the swaps are renames, not copies. Ask for the staging - # copy plus 20% headroom rather than a full 2x, which would block - # updates that would have succeeded on exactly the space-constrained - # machines most likely to hit this path. + # Swaps are renames, so only the staging copy is new: require it plus + # 20% headroom, not 2x — which would block updates on exactly the + # space-constrained machines most likely to hit this path. required = int(need * 1.2) free = shutil.disk_usage(str(_m().PROJECT_ROOT)).free if free < required: @@ -2314,12 +2196,10 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo src = os.path.join(extracted, item) dst = os.path.join(str(_m().PROJECT_ROOT), item) staged.append((_stage_replacement(src, dst), dst)) - # #70337/#87331: the GitHub source ZIP contains only source — - # apps/desktop/release/ (the BUILT desktop app, win-unpacked/ - # Hermes.exe) exists only in the LIVE tree. Swapping `apps` - # without it deletes the desktop build and breaks the - # shortcut. Graft the live release dir into the staged copy - # BEFORE the swap so the commit preserves it atomically. + # #70337/#87331: the source ZIP lacks apps/desktop/release/ + # (the BUILT desktop app); swapping `apps` without it deletes + # the build and breaks the shortcut. Graft the live release + # dir into the staged copy BEFORE the swap. if item == "apps": live_release = os.path.join(dst, "desktop", "release") staged_release = os.path.join( @@ -2337,11 +2217,9 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo raise try: - # Re-check the tree right before the swap (#87304 TOCTOU): the - # download + extract + staging window above can take minutes, and - # work created in it would be destroyed by the commit below. Our - # own phase-1 staging siblings are filtered out — they are the - # expected artifacts of getting here, not user work. + # Re-check right before the swap (#87304 TOCTOU): download + + # extract + staging can take minutes, and work created meanwhile + # would be destroyed. Our own staging siblings are filtered out. recheck_reason = _zip_overlay_block_reason( _m().PROJECT_ROOT, ignore_staging_artifacts=True ) @@ -2356,15 +2234,11 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo _m().sys.exit(1) _commit_staged_replacements(staged) except Exception: - # The rollback already restored every swapped entry, but staging - # copies for the not-yet-swapped entries (potentially most of a - # full tree) are still on disk. Drop them, or the retry's - # up-front free-space check — which runs BEFORE the lazy - # per-entry leftover cleanup — fails on litter this attempt - # left behind: the exact "retry fails harder" failure mode - # _discard_staged exists to prevent. Safe post-rollback: swapped - # entries' staging paths were renamed away, and _discard_staged - # skips paths that no longer exist. + # Rollback restored the swapped entries, but staging copies for + # the rest (possibly most of a tree) remain. Drop them, or the + # retry's up-front free-space check (which runs BEFORE per-entry + # leftover cleanup) fails on our litter. Safe post-rollback: + # _discard_staged skips paths that no longer exist. _discard_staged(staged) raise update_count = len(staged) @@ -2385,21 +2259,12 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo finally: shutil.rmtree(tmp_dir, ignore_errors=True) - # Clear stale bytecode after ZIP extraction - removed = _m()._clear_bytecode_cache(_m().PROJECT_ROOT) - if removed: - print( - f" ✓ Cleared {removed} stale __pycache__ director{'y' if removed == 1 else 'ies'}" - ) - _m()._record_bytecode_fingerprint() - _m()._refresh_bootstrap_cache_scripts(branch) + _sweep_bytecode_after_update(branch) - # Reinstall Python dependencies. Prefer .[all], but if one optional extra - # breaks on this machine, keep base deps and reinstall the remaining extras - # individually so update does not silently strip working capabilities. - # - # Self-lock deferral (relocated preflight — #86735): the ZIP code swap - # above is already committed; defer only the dependency sync when this + # Reinstall Python deps: prefer .[all]; if one extra breaks, keep base + # deps and retry the remaining extras individually so working + # capabilities aren't silently stripped. Self-lock deferral (#86735): the + # code swap is committed; defer only the dependency sync when this # process holds a native extension the sync must rewrite. _m()._abort_dependency_sync_if_self_locked() print("→ Updating Python dependencies...") @@ -2433,23 +2298,8 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo # here with the same defer-via-marker contract. _refuse_update_for_contended_shims(_sqe) else: - # Use sys.executable to explicitly call the venv's pip module, - # avoiding PEP 668 'externally-managed-environment' errors on Debian/Ubuntu. - # Some environments lose pip inside the venv; bootstrap it back with - # ensurepip before trying the editable install. - try: - subprocess.run( - pip_cmd + ["--version"], - cwd=_m().PROJECT_ROOT, - check=True, - capture_output=True, - ) - except subprocess.CalledProcessError: - subprocess.run( - [_m().sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], - cwd=_m().PROJECT_ROOT, - check=True, - ) + # sys.executable -m pip avoids PEP 668 'externally-managed-environment' errors. + _ensure_venv_pip(pip_cmd, _m().sys.executable) _m()._install_python_dependencies_with_optional_fallback(pip_cmd) install_prefix = [uv_bin, "pip"] if uv_bin else pip_cmd @@ -2465,15 +2315,11 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo # #70636). _m()._refresh_active_memory_provider_dependencies() - # Now that dependencies are installed, verify the tree actually imports. - # The copy loop above replaces top-level entries one at a time in - # os.listdir order, so an interruption between (say) `agent/` and `tools/` - # leaves a tree whose files all parse but cannot be imported together — - # the ImportError-on-startup class this guard exists to catch. Deliberately - # placed *after* the dependency reinstall so a genuinely-new third-party - # requirement isn't misreported as a partial copy. There is no SHA to roll - # back to here, so surface it with a concrete recovery step rather than - # reporting a successful update over a bricked install. + # Verify the tree actually imports (catches the parse-OK-but-skewed tree + # an interrupted copy leaves). Placed *after* the dependency reinstall so + # a genuinely-new third-party requirement isn't misreported as a partial + # copy. No SHA to roll back to here — surface a concrete recovery step + # instead of reporting success over a bricked install. import_ok, failing_module, import_error = _validate_critical_modules_import( _m().PROJECT_ROOT ) @@ -2493,33 +2339,9 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo had_desktop_app_before_update=had_desktop_app_before_update, ) - # Sync skills try: - from tools.skills_sync import sync_skills - print("→ Syncing bundled skills...") - result = sync_skills(quiet=True) - if result["copied"]: - print(f" + {len(result['copied'])} new: {', '.join(result['copied'])}") - if result.get("updated"): - print( - f" ↑ {len(result['updated'])} updated: {', '.join(result['updated'])}" - ) - if result.get("user_modified"): - print(f" ~ {len(result['user_modified'])} user-modified (kept)") - print( - " → see them: hermes skills list-modified " - "(diff/reset to resume updates)" - ) - if result.get("cleaned"): - print(f" − {len(result['cleaned'])} removed from manifest") - if result.get("relocated"): - print( - f" → {len(result['relocated'])} moved to new upstream paths: " - f"{', '.join(result['relocated'])}" - ) - if not result["copied"] and not result.get("updated"): - print(" ✓ Skills are up to date") + _print_bundled_skills_sync_report() except Exception: pass @@ -2533,10 +2355,8 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo except Exception as e: logger.debug("Model catalog seed during zip update failed: %s", e) - # ── Post-update state.db integrity guard (#68474, #97994) ──────────── - # Verify state.db survived the ZIP update in the root home AND every - # sibling profile, auto-restoring each from its own most recent valid - # pre-update snapshot when needed. + # Post-update state.db integrity guard (#68474, #97994): root home AND + # every sibling profile, each auto-restored from its own snapshot. try: _verify_and_restore_state_dbs_post_update() except Exception as exc: @@ -2571,13 +2391,7 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo return update_complete def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[str]: - status = subprocess.run( - git_cmd + ["status", "--porcelain"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - check=True, - ) + status = _git_run(git_cmd, ["status", "--porcelain"], cwd, check=True) if not status.stdout.strip(): return None @@ -2585,12 +2399,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st # git stash will fail with "needs merge / could not write index". Clear the # conflict state with `git reset` so the stash can proceed. Working-tree # changes are preserved; only the index conflict markers are dropped. - unmerged = subprocess.run( - git_cmd + ["ls-files", "--unmerged"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + unmerged = _git_run(git_cmd, ["ls-files", "--unmerged"], cwd) if unmerged.stdout.strip(): print("→ Clearing unmerged index entries from a previous conflict...") subprocess.run(git_cmd + ["reset"], cwd=cwd, capture_output=True) @@ -2601,26 +2410,11 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st f"{_AUTOSTASH_NAME_PREFIX}%Y%m%d-%H%M%S" ) print("→ Local changes detected — stashing before update...") - prev_stash = subprocess.run( - git_cmd + ["rev-parse", "--verify", "refs/stash"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ).stdout.strip() - push = subprocess.run( - git_cmd + ["stash", "push", "--include-untracked", "-m", stash_name], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + prev_stash = _git_run(git_cmd, ["rev-parse", "--verify", "refs/stash"], cwd).stdout.strip() + push = _git_run(git_cmd, ["stash", "push", "--include-untracked", "-m", stash_name], cwd) if push.stdout.strip(): print(push.stdout.strip()) - stash_probe = subprocess.run( - git_cmd + ["rev-parse", "--verify", "refs/stash"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + stash_probe = _git_run(git_cmd, ["rev-parse", "--verify", "refs/stash"], cwd) stash_ref = stash_probe.stdout.strip() stash_created = ( stash_probe.returncode == 0 and bool(stash_ref) and stash_ref != prev_stash @@ -2628,12 +2422,10 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st if push.returncode != 0: if stash_created: - # git stash push exits non-zero when it saved everything but could - # not delete some swept untracked files from the working tree - # (e.g. a root-owned directory: "warning: failed to remove ...: - # Permission denied"). The stash entry is complete — the changes - # are safe — so this is not a failure. Leave the undeletable - # files in place and continue the update. + # stash push exits non-zero when it saved everything but couldn't + # delete some swept untracked files (e.g. a root-owned dir: + # "failed to remove ...: Permission denied"). The entry is + # complete, so not a failure — leave the files and continue. if push.stderr.strip(): print(push.stderr.strip()) print( @@ -2672,13 +2464,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st def _resolve_stash_selector( git_cmd: list[str], cwd: Path, stash_ref: str ) -> Optional[str]: - stash_list = subprocess.run( - git_cmd + ["stash", "list", "--format=%gd %H"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - check=True, - ) + stash_list = _git_run(git_cmd, ["stash", "list", "--format=%gd %H"], cwd, check=True) for line in stash_list.stdout.splitlines(): selector, _, commit = line.partition(" ") if commit.strip() == stash_ref: @@ -2700,26 +2486,18 @@ _AUTOSTASH_WARN_AGE_DAYS = 7 def _warn_orphaned_update_autostashes(git_cmd: list[str], cwd: Path) -> int: """Surface leftover update autostashes older than the warn threshold. - Autostash entries legitimately outlive an update run (``--keep-stash`` - parks them; a conflicted or failed restore preserves them for safety), but - nothing ever re-surfaces them afterwards — they sit in ``git stash`` - invisibly for weeks (#63717 problem 6). This prints a short notice naming - the stale entries with recovery/cleanup guidance. Deliberately NOT a GC: - a stash entry can be the only copy of the user's uncommitted work, so - Hermes never drops one automatically. + Autostashes legitimately outlive a run (``--keep-stash`` parks them; a + failed restore preserves them), but nothing re-surfaces them — they sit + invisibly for weeks (#63717 problem 6). Prints a notice with recovery/ + cleanup guidance. Deliberately NOT a GC: a stash entry can be the only + copy of the user's uncommitted work, so Hermes never drops one. - Best-effort — any git failure returns 0 and must not block the update. - Returns the number of stale entries warned about. + Best-effort — any git failure returns 0. Returns the stale-entry count. """ from datetime import timedelta, timezone try: - stash_list = subprocess.run( - git_cmd + ["stash", "list", "--format=%gd %s"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + stash_list = _git_run(git_cmd, ["stash", "list", "--format=%gd %s"], cwd) if stash_list.returncode != 0: return 0 cutoff = datetime.now(timezone.utc) - timedelta( @@ -2963,11 +2741,8 @@ def _restore_stashed_changes( try: response = input().strip().lower() except (EOFError, UnicodeDecodeError): - # Mirror the config-migration prompt's fix: don't let a - # terminal-encoding issue or a closed stdin crash the - # update mid-restore. Falls through to the existing - # skip-restore path below, which already explains how to - # restore manually from git stash. + # A closed stdin or terminal-encoding error must not crash the + # update mid-restore; fall through to the skip-restore path. response = "n" accepted = response in {"y", "yes"} or (not remote_prompt and response == "") if not accepted: @@ -2985,30 +2760,18 @@ def _restore_stashed_changes( cwd, report_runtime_errors=True ) print("→ Restoring local changes...") - restore = subprocess.run( - git_cmd + ["stash", "apply", stash_ref], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + restore = _git_run(git_cmd, ["stash", "apply", stash_ref], cwd) # Check for unmerged (conflicted) files — can happen even when returncode is 0 - unmerged = subprocess.run( - git_cmd + ["diff", "--name-only", "--diff-filter=U"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + unmerged = _git_run(git_cmd, ["diff", "--name-only", "--diff-filter=U"], cwd) has_conflicts = bool(unmerged.stdout.strip()) if restore.returncode != 0 and not has_conflicts and ( _stash_apply_failed_only_on_existing_untracked(restore.stderr) ): - # Permission-denied autostash tail end: the tracked changes applied - # cleanly; the only "failure" is untracked files that never left the - # working tree (git could not delete them at stash time, so it now - # refuses to overwrite them). Their content was never touched — - # nothing is lost. Treat as restored. + # Tracked changes applied cleanly; the only "failure" is untracked files + # git couldn't delete at stash time and now refuses to overwrite. Their + # content is untouched — treat as restored. print( " ⚠ Some stashed untracked files already exist in the working " "tree and were kept as-is." @@ -3020,7 +2783,6 @@ def _restore_stashed_changes( if restore.stderr.strip(): print(restore.stderr.strip()) - # Show which files conflicted conflicted_files = unmerged.stdout.strip() if conflicted_files: print("\nConflicted files:") @@ -3030,9 +2792,8 @@ def _restore_stashed_changes( print("\nYour stashed changes are preserved — nothing is lost.") print(f" Stash ref: {stash_ref}") - # Always reset to clean state — leaving conflict markers in source - # files makes hermes completely unrunnable (SyntaxError on import). - # The user's changes are safe in the stash for manual recovery. + # Always reset: conflict markers in source make hermes unrunnable + # (SyntaxError on import). The user's changes remain in the stash. subprocess.run( git_cmd + ["reset", "--hard", "HEAD"], cwd=cwd, @@ -3040,9 +2801,8 @@ def _restore_stashed_changes( ) print("Working tree reset to clean state.") print(f"Restore your changes later with: git stash apply {stash_ref}") - # Don't sys.exit — the code update itself succeeded, only the stash - # restore had conflicts. Let cmd_update continue with pip install, - # skill sync, and gateway restart. + # Don't exit: the code update succeeded; let cmd_update continue with + # pip install, skill sync, and gateway restart. return False restored_python = _restored_python_paths(git_cmd, cwd) @@ -3100,12 +2860,7 @@ def _restore_stashed_changes( ) _print_stash_cleanup_guidance(stash_ref) else: - drop = subprocess.run( - git_cmd + ["stash", "drop", stash_selector], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + drop = _git_run(git_cmd, ["stash", "drop", stash_selector], cwd) if drop.returncode != 0: print( "⚠ Local changes were restored, but Hermes couldn't drop the saved stash entry." @@ -3128,19 +2883,14 @@ def _discard_stashed_changes( cwd: Path, stash_ref: str, ) -> bool: - """Throw away a stash created before an update, without applying it. + """Drop a pre-update stash without applying it. - Used only on a NON-interactive update when the user has set - ``updates.non_interactive_local_changes: discard`` — i.e. they've opted out - of keeping local source edits on this machine. Drops the stash entry - instead of re-applying it, so the working tree stays clean at the freshly - pulled HEAD. Unlike ``git reset --hard`` + ``git clean -fd``, this only - affects what was stashed (tracked changes + the untracked files we - explicitly captured) — ignored paths like node_modules/venv/build outputs - are never touched, since they were never stashed. + Only for NON-interactive updates with + ``updates.non_interactive_local_changes: discard``. Unlike ``git reset + --hard`` + ``git clean -fd``, this touches only what was stashed — ignored + paths (node_modules, venv, build outputs) are never affected. - Returns True if the stash was dropped, False on a git failure (in which - case the stash is left in place for safety). + Returns True if dropped, False on git failure (stash left in place). """ stash_selector = _resolve_stash_selector(git_cmd, cwd, stash_ref) if stash_selector is None: @@ -3151,12 +2901,7 @@ def _discard_stashed_changes( _print_stash_cleanup_guidance(stash_ref) return False - drop = subprocess.run( - git_cmd + ["stash", "drop", stash_selector], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + drop = _git_run(git_cmd, ["stash", "drop", stash_selector], cwd) if drop.returncode != 0: print( "⚠ Configured to discard local changes, but Hermes couldn't drop " @@ -3184,12 +2929,7 @@ SKIP_UPSTREAM_PROMPT_FILE = ".skip_upstream_prompt" def _get_origin_url(git_cmd: list[str], cwd: Path) -> Optional[str]: """Get the URL of the origin remote, or None if not set.""" try: - result = subprocess.run( - git_cmd + ["remote", "get-url", "origin"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + result = _git_run(git_cmd, ["remote", "get-url", "origin"], cwd) if result.returncode == 0: return result.stdout.strip() except Exception: @@ -3215,12 +2955,7 @@ def _is_fork(origin_url: Optional[str]) -> bool: def _has_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: """Check if an 'upstream' remote already exists.""" try: - result = subprocess.run( - git_cmd + ["remote", "get-url", "upstream"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + result = _git_run(git_cmd, ["remote", "get-url", "upstream"], cwd) return result.returncode == 0 except Exception: return False @@ -3228,12 +2963,7 @@ def _has_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: def _add_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: """Add the official repo as the 'upstream' remote. Returns True on success.""" try: - result = subprocess.run( - git_cmd + ["remote", "add", "upstream", OFFICIAL_REPO_URL], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + result = _git_run(git_cmd, ["remote", "add", "upstream", OFFICIAL_REPO_URL], cwd) return result.returncode == 0 except Exception: return False @@ -3241,12 +2971,7 @@ def _add_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: def _count_commits_between(git_cmd: list[str], cwd: Path, base: str, head: str) -> int: """Count commits on `head` that are not on `base`. Returns -1 on error.""" try: - result = subprocess.run( - git_cmd + ["rev-list", "--count", f"{base}..{head}"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + result = _git_run(git_cmd, ["rev-list", "--count", f"{base}..{head}"], cwd) if result.returncode == 0: return int(result.stdout.strip()) except Exception: @@ -3274,13 +2999,7 @@ def _sync_fork_with_upstream(git_cmd: list[str], cwd: Path) -> bool: Returns True if push succeeded, False otherwise. """ try: - result = subprocess.run( - git_cmd + ["push", "origin", "main", "--force-with-lease"], - cwd=cwd, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - **_no_prompt_git_kwargs(), - ) + result = _git_run(git_cmd, ["push", "origin", "main", "--force-with-lease"], cwd, network=True) return result.returncode == 0 except Exception: return False @@ -3292,24 +3011,19 @@ def _sync_with_upstream_if_needed( assume_yes: bool = False, input_fn=None, ) -> bool: - """Check if fork is behind upstream and sync if safe. + """Check if fork is behind upstream and fast-forward if safe. - This implements the fork upstream sync logic: - - If upstream remote doesn't exist, ask user if they want to add it - - Compare origin/main with upstream/main - - If origin/main is strictly behind upstream/main, pull from upstream - - Try to sync fork back to origin if possible + Offers to add the ``upstream`` remote, compares origin/main with + upstream/main, pulls when strictly behind, then tries to push origin. - Returns True when origin/main was actually verified against the official - upstream/main, False when the check never happened (prompt skipped or - declined, remote add failed, fetch or compare failed) so the caller can - avoid reporting the checkout as up to date on the strength of an origin - comparison alone (#97052 review). + Returns True only when origin/main was actually verified against + upstream/main; False when the check never happened (prompt declined, + remote add/fetch/compare failed) so the caller never reports "up to date" + on an origin-only comparison (#97052). """ has_upstream = _has_upstream_remote(git_cmd, cwd) if not has_upstream: - # Check if user previously declined if _should_skip_upstream_prompt(): return False @@ -3329,7 +3043,6 @@ def _sync_with_upstream_if_needed( ) return False - # Ask user if they want to add upstream if input_fn is not None: response = ( input_fn("Add official repo as 'upstream' remote? [y/N]", "n") @@ -3364,9 +3077,8 @@ def _sync_with_upstream_if_needed( _mark_skip_upstream_prompt() return False - # Fetch upstream main only. This sync compares upstream/main with - # origin/main, so there's no reason to pull every upstream ref — and a bare - # fetch drags in thousands of auto-generated branches. + # Fetch only upstream/main: a bare fetch drags in thousands of + # auto-generated branches. print() print("→ Fetching upstream...") try: @@ -3400,7 +3112,6 @@ def _sync_with_upstream_if_needed( print(" git pull upstream main") return True - # If upstream is not ahead, fork is up to date if upstream_ahead == 0: print(" ✓ Fork is up to date with upstream") return True @@ -3425,7 +3136,6 @@ def _sync_with_upstream_if_needed( print(" ✓ Updated from upstream") - # Try to sync fork back to origin print("→ Syncing fork...") if _sync_fork_with_upstream(git_cmd, cwd): print(" ✓ Fork synced with upstream") @@ -3437,11 +3147,10 @@ def _sync_with_upstream_if_needed( return True def _invalidate_update_cache(): - """Delete the update-check cache for ALL profiles so no banner - reports a stale "commits behind" count after a successful update. + """Delete the update-check cache for ALL profiles. - The git repo is shared across profiles — when one profile runs - ``hermes update``, every profile is now current. + The git repo is shared, so one profile's update makes every profile + current; a per-profile cache would show a stale "commits behind" banner. """ homes = [] # Default profile home (Docker-aware — uses /opt/data in Docker) @@ -3484,11 +3193,9 @@ def _write_lazy_refresh_incomplete_marker() -> None: _write_marker_file(_m()._lazy_refresh_marker_path(), label="lazy-refresh-incomplete") -# ``fleet_restart_pending`` lives under HERMES_HOME (not next to the venv). -# The existing ``.update-incomplete`` / ``.lazy-refresh-incomplete`` markers -# gate dependency/venv repair; this one is the fleet-restart obligation after -# a git pull that advanced HEAD (#95294). Cleared only when the restart phase -# completes or there were no running services to restart. +# Lives under HERMES_HOME (not next to the venv). Unlike the venv-repair +# markers, this records the fleet-restart obligation after a pull advanced +# HEAD (#95294); cleared only when the restart completes or nothing was running. _FLEET_RESTART_PENDING_NAME = "fleet_restart_pending" @@ -3549,12 +3256,10 @@ def _receipt_looks_unfinished(receipt: dict) -> bool: def _receipt_reports_stale_runtime(expected_sha: str | None = None) -> bool: """True when ``update_receipts/latest.json`` records a runtime SHA skew. - ``plan.runtimes[].code_sha`` is captured *before* the pull of that run, - so a successful update's receipt always shows pre-update runtime SHAs. - Those must not retrigger a restart on the next invocation. Use the - post-restart ``fleet`` matrix when present; fall back to the plan only - for an unfinished receipt (interrupt / failed / incomplete restart) — - the #95294 smoking-gun shape. + Prefer the post-restart ``fleet`` matrix. ``plan.runtimes[].code_sha`` is + captured *before* the pull, so a finished update's plan always shows stale + SHAs and must not retrigger a restart; consult it only for an unfinished + receipt (#95294). """ try: from hermes_cli.update_receipt import read_latest_receipt @@ -3640,20 +3345,9 @@ def _restart_systemd_gateway_units_best_effort(failed: list) -> None: ("system", ["systemctl"]), ): try: - result = subprocess.run( - scope_cmd - + [ - "list-units", - "hermes-gateway*", - "hermes-serve*", - "--plain", - "--no-legend", - "--no-pager", - ], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", + result = _systemctl( + scope_cmd + ["list-units", "hermes-gateway*", "hermes-serve*", + "--plain", "--no-legend", "--no-pager"], timeout=10, ) except (FileNotFoundError, subprocess.TimeoutExpired): @@ -3669,14 +3363,7 @@ def _restart_systemd_gateway_units_best_effort(failed: list) -> None: and os.geteuid() != 0 # windows-footgun: ok — systemd path, Linux-only ): restart_cmd = ["sudo", "-n"] + restart_cmd - subprocess.run( - restart_cmd, - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - timeout=30, - ) + _systemctl(restart_cmd, timeout=30) def on_timeout(svc_name: str, exc: subprocess.TimeoutExpired) -> None: failed.append(svc_name) @@ -3815,22 +3502,16 @@ def _format_concurrent_instances_message( def _classify_concurrent_instance(pid: int) -> str: """Return ``"gateway"`` when ``pid``'s command line is a gateway runtime. - Delegates to ``_is_pausable_gateway`` — the same canonical - ``gateway run`` matcher (``gateway.status.looks_like_gateway_command_line``, - shlex-tokenized, profile-selector aware) used by the Desktop preflight - exemption and the venv-holder guard fallback — so a PID classified as - ``"gateway"`` here is exactly the set the pause/kill+restart machinery - downstream will stop. That symmetry is what lets the pre-update - concurrent gate skip the abort for gateway-only matches: the gateway is - going to be stopped by ``_pause_windows_gateways_for_update()`` moments - later anyway, so refusing the update just to make the user kill it - manually is friction without benefit. + Delegates to ``_is_pausable_gateway`` — the same canonical ``gateway run`` + matcher used by the Desktop preflight exemption and the venv-holder guard + — so a PID classified ``"gateway"`` here is exactly the set the downstream + pause/kill+restart machinery will stop. That symmetry lets the pre-update + concurrent gate skip the abort for gateway-only matches instead of making + the user kill a gateway that is about to be paused anyway. - Returns ``"non-gateway"`` when the cmdline doesn't match, and - ``"unknown"`` when psutil can't read it (process gone, access denied, - psutil missing). The gate treats ``"unknown"`` as non-gateway — we'd - rather block an update we could have completed than proceed against a - process we couldn't positively identify as a gateway. + Returns ``"non-gateway"`` when the cmdline doesn't match and ``"unknown"`` + when psutil can't read it; the gate treats ``"unknown"`` as non-gateway + (better to block than proceed against an unidentified process). """ try: import psutil # noqa: PLC0415 @@ -3856,13 +3537,10 @@ def _filter_non_gateway_concurrent_instances( ) -> list[tuple[int, str]]: """Return only the concurrent-instance matches that are NOT the gateway. - Used by the pre-update concurrent gate to decide whether to abort - ``hermes update``. If every concurrent instance is a gateway, the pause - machinery (``_pause_windows_gateways_for_update``) and the post-update - kill+restart block handle it — the update proceeds. If anything else (a - TUI shell, a Hermes Desktop backend child, an unrelated ``hermes`` REPL) - is in the list, the gate still aborts with the existing message, since - those have no pause machinery downstream. + If every concurrent instance is a gateway, the pause machinery and the + post-update kill+restart handle it and the update proceeds. Anything else + (TUI shell, Desktop backend child, another ``hermes`` REPL) has no pause + machinery downstream, so the gate still aborts. """ non_gateway: list[tuple[int, str]] = [] for pid, name in matches: @@ -3978,9 +3656,8 @@ def _restore_active_tool_dependencies( ) restored.append(name) except Exception as exc: - # This is best-effort recovery for optional tooling. Unexpected - # installer failures must be surfaced without aborting the core - # runtime update. + # Best-effort optional tooling: surface failures without aborting + # the core update. failed.append((name, str(exc))) if restored: @@ -3999,21 +3676,14 @@ def _refresh_active_lazy_features( ) -> bool: """Refresh lazy-installed backends after a code update. - When pyproject.toml's ``[all]`` extra was slimmed down (May 2026), most - optional backends moved to ``tools/lazy_deps.py`` and only install on - first use. ``hermes update`` runs ``uv pip install -e .[all]`` which - leaves those packages untouched — so if we bump a pin in - :data:`LAZY_DEPS` (CVE response, transitive bug fix), users who already - activated the backend keep the stale version forever. + ``uv pip install -e .[all]`` never touches ``tools/lazy_deps.py`` backends, + so a bumped :data:`LAZY_DEPS` pin (CVE, transitive fix) would otherwise + leave already-activated backends stale forever. Reinstalls only the + features the user previously activated; cold backends stay untouched. - This function asks lazy_deps which features the user has previously - activated and reinstalls them under the current pins. Features the - user never enabled stay quiet — no churn for cold backends. - - Returns True when the venv is safe to use (refresh succeeded, or no - active lazy backends, or post-failure import repair succeeded). Returns - False when a failed lazy install left broken core imports that automatic - repair could not fix (#57828). + Returns True when the venv is safe to use (refresh succeeded, nothing + active, or post-failure import repair succeeded); False when a failed + lazy install left broken core imports that repair could not fix (#57828). Never raises. A failure here must not block the rest of the update. """ @@ -4105,14 +3775,11 @@ def _refresh_active_lazy_features( def _refresh_active_memory_provider_dependencies() -> None: """Refresh pip dependencies for the configured external memory provider. - Memory-provider bridge packages are declared in each provider's - ``plugin.yaml`` (plus mode-dependent extras like Hindsight's - ``hindsight-all``), NOT in Hermes' editable-install extras or - ``LAZY_DEPS`` alone — so the core dependency reinstall above can strip - or downgrade them (#53272 mem0ai, #70636 hindsight-embed). Re-run the - provider's declared install for the ACTIVE provider only, after the - core install and lazy refresh, so the last write to any shared package - is the one the active provider needs. + Provider bridge packages are declared in each provider's ``plugin.yaml`` + (plus mode extras like Hindsight's ``hindsight-all``), not in Hermes' + extras or ``LAZY_DEPS``, so the core reinstall can strip or downgrade + them (#53272, #70636). Re-run the ACTIVE provider's install after the + core install and lazy refresh so its writes to shared packages land last. Never raises. A failure here must not block the rest of the update. """ @@ -4160,18 +3827,12 @@ def _install_psutil_android_compat( ) -> None: """Install psutil on Android by patching upstream platform detection. - psutil's setup currently gates Linux sources behind - ``sys.platform.startswith('linux')``. On Termux Python reports - ``sys.platform == 'android'``, so setup aborts with - "platform android is not supported" despite compiling fine when using the - Linux source path. + psutil's setup gates Linux sources behind ``sys.platform.startswith('linux')``; + Termux reports ``'android'``, so setup aborts although the Linux source path + compiles fine. Only the extracted build tree for this attempt is patched. - We patch only the extracted build tree used for this install attempt; - nothing is persisted in the repository. - - Stopgap: remove this once https://github.com/giampaolo/psutil/pull/2762 - merges and ships in a release. The standalone installer script uses the - same shared helper and should be removed together. + Stopgap: remove (together with the standalone installer's use of the same + helper) once https://github.com/giampaolo/psutil/pull/2762 ships. """ import tempfile import urllib.request @@ -4191,11 +3852,9 @@ def _install_psutil_android_compat( def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None: """Best-effort uv bootstrap on Termux for faster update installs. - The normal path (``ensure_uv()`` in managed_uv) installs the managed - standalone uv into ``$HERMES_HOME/bin/uv``, but on Termux the official - installer may not work (glibc vs bionic). Prefer a uv already on PATH - (e.g. ``pkg install uv``); only if there is none do we fall back to a - wheel-only ``pip install uv`` so we never source-build the Rust crate. + The official uv installer may not work on Termux (glibc vs bionic). Prefer + a uv already on PATH (``pkg install uv``); otherwise fall back to a + wheel-only ``pip install uv`` so the Rust crate is never source-built. """ from hermes_cli.managed_uv import resolve_uv @@ -4221,27 +3880,22 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None: return None except Exception: pass - # After pip install, check managed path first, then PATH return resolve_uv() or shutil.which("uv") def _npm_manifest_paths() -> tuple[Path, ...]: """Manifests whose changes must defeat the update-skip. - The lockfile alone is NOT a sufficient key: on a local checkout a dev - can edit package.json (root or a workspace) without running npm — the - lockfile is then unchanged but `hermes update` is exactly the step - expected to sync node_modules (via the `npm install` fallback in + The lockfile alone is not a sufficient key: a dev can edit a package.json + (root or workspace) without running npm, and `hermes update` is exactly + the step expected to sync node_modules (`npm install` fallback in _run_npm_install_deterministic). - The workspace list is pulled from the root package.json's `workspaces` - globs (npm's own source of truth) rather than hardcoded, so adding a - workspace can never silently escape the skip key. Every workspace - manifest belongs in the key — desktop included, even though the - install only names ui-tui and web — because the single lockfile spans - the whole workspace graph, so any manifest edit can put the lockfile - out of sync and change what the install must do. Falls back to hashing - just root manifests if package.json is unreadable (never skips more - than main would have installed). + Workspaces come from the root package.json's `workspaces` globs so a new + workspace can never escape the key. Every workspace manifest counts — + desktop included, though the install names only ui-tui and web — because + the single lockfile spans the whole workspace graph. Falls back to root + manifests only if package.json is unreadable (never skips more than main + would have installed). """ root_pkg = _m().PROJECT_ROOT / "package.json" paths = [_m().PROJECT_ROOT / "package-lock.json", root_pkg] @@ -4284,10 +3938,8 @@ def _npm_lockfile_changed(hermes_root: Path) -> bool: # node_modules means the cache was recorded by another checkout. if not (_m().PROJECT_ROOT / "node_modules").is_dir(): return True - # A matching lockfile hash over a tree whose web build toolchain never - # landed must NOT skip the reinstall — otherwise every later `hermes - # update` keeps rebuilding against a half-installed tree and serving a - # stale dist. + # A matching hash must NOT skip the reinstall when the web build toolchain + # never landed, or every later update rebuilds against a half-installed tree. web_dir = _m().PROJECT_ROOT / "web" if (web_dir / "package.json").is_file() and not _web_build_toolchain_ready( *_web_toolchain_roots(web_dir) @@ -4325,15 +3977,12 @@ def _repair_node_deps_on_current_checkout( ) -> bool: """Repair Node deps on the ``commit_count == 0`` path (#77211). - A current checkout does not imply healthy Node deps: a previous npm - install may have failed (EBADENGINE from a node/npm mismatch, network - timeout, interrupted install) and its error message says to "re-run - hermes update" — but the early return never reached the Node refresh, - so that repair advice could never work. ``_update_node_dependencies`` - self-gates on the lockfile hash, which is only recorded after a - SUCCESSFUL npm install (and re-trips when node_modules is missing or - the web toolchain never landed), so this is a cheap no-op on healthy - installs and a real repair after a failed one. + A current checkout does not imply healthy Node deps: a failed npm install + (EBADENGINE, network timeout, interrupt) says "re-run hermes update", but + the early return never reached the Node refresh. ``_update_node_dependencies`` + self-gates on the lockfile hash, recorded only after a SUCCESSFUL install + (and re-tripped when node_modules or the web toolchain is missing), so this + is a cheap no-op on healthy installs and a real repair after a failed one. """ node_failures = _update_node_dependencies() if node_failures: @@ -4352,12 +4001,9 @@ def _repair_node_deps_on_current_checkout( gateway_mode=gateway_mode, pre_update_snapshot_id=pre_update_snapshot_id, ) - # A current checkout can still owe a Desktop rebuild (#97343): the - # packaged app is built from source the pull already landed — or, on the - # Windows hand-off, by a child that never reaches the commits-pulled - # rebuild. Skipping it leaves a stale desktop app behind a - # successful-looking update. Self-gates on the build stamp, so this is a - # no-op when nothing changed. + # A current checkout can still owe a Desktop rebuild (#97343) — e.g. the + # Windows hand-off child never reaches the commits-pulled rebuild — leaving + # a stale app behind a successful-looking update. Self-gates on the build stamp. if not _rebuild_desktop_after_update( _m().PROJECT_ROOT / "apps" / "desktop", had_desktop_app_before_update=had_desktop_app_before_update, @@ -4406,15 +4052,13 @@ def _update_node_dependencies() -> list[str]: from hermes_constants import get_default_hermes_root - # This cache describes PROJECT_ROOT/node_modules, which is shared by every - # Hermes profile using this checkout. Keep one per-checkout cache under the - # shared Hermes root rather than rerunning npm once per named profile. + # node_modules is shared by every profile on this checkout, so keep one + # per-checkout cache under the shared root instead of one per profile. shared_hermes_root = get_default_hermes_root() - # Best-effort: warm npx's cache for agent-browser (#43564). Runs before - # the lockfile-unchanged early return below since that's the common - # `hermes update` case. Synchronous and can block ~11s on a true cold - # cache (~0.4s once warm) — print first so that doesn't look like a hang. + # Best-effort npx cache warm for agent-browser (#43564), before the + # lockfile-unchanged early return (the common case). Can block ~11s on a + # cold cache — print first so it doesn't look like a hang. print("→ Warming npx cache for agent-browser...") try: from tools.browser_tool import warm_agent_browser_npx_cache @@ -4426,15 +4070,11 @@ def _update_node_dependencies() -> list[str]: logger.info("npm lockfile unchanged, skipping npm install") return [] - # Root package.json has no dependencies of its own (agent-browser and - # @streamdown/math were moved out — see #43564): agent-browser resolves - # at runtime via `npx agent-browser` (tools/browser_tool.py), and - # @streamdown/math is a desktop-only import now declared in - # apps/desktop/package.json. That means a plain workspace-scoped install - # can never prune anything root-only, so we only need to name the - # workspaces the CLI/TUI/web build actually requires. apps/desktop pulls - # in Electron as a devDependency with a ~200MB postinstall download, so - # it's deliberately never named here — desktop deps install on demand + # Root package.json has no dependencies of its own (#43564: agent-browser + # resolves via `npx` at runtime, @streamdown/math moved to apps/desktop), + # so a workspace-scoped install prunes nothing root-only. apps/desktop is + # deliberately never named: its Electron devDependency has a ~200MB + # postinstall download, so desktop deps install on demand # (see _desktop_build_needed). print("→ Updating Node.js dependencies...") @@ -4448,12 +4088,10 @@ def _update_node_dependencies() -> list[str]: install_args = [ "--no-fund", "--no-audit", "--prefer-offline", "--progress=false", "--workspace", "ui-tui", "--workspace", "web", - # Root package.json's own devDependencies (the shared ESLint flat - # config every workspace's eslint.config.mjs imports) are otherwise - # pruned by this scoped install, same as agent-browser/@streamdown - # math used to be before they moved out of root entirely (#43564). - # Unlike those, root's devDependencies have nowhere else to live — - # this flag still excludes apps/desktop, which is never named above. + # Root's own devDependencies (the shared ESLint flat config every + # workspace imports) would otherwise be pruned by this scoped install + # and have nowhere else to live. apps/desktop is still excluded since + # it is never named above. "--include-workspace-root", ] @@ -4461,11 +4099,10 @@ def _update_node_dependencies() -> list[str]: nixos_env = with_hermes_node_path(_m()._nixos_build_env()) - # NOTE: capture_output=False here is deliberate (#18840) — optional - # postinstall scripts print download progress, and capturing it makes a - # long download look hung. The chatty npm-deprecation noise during - # `hermes update` comes from the *desktop* build, not this step; that - # one is captured to update.log. + # capture_output=False is deliberate (#18840): optional postinstall scripts + # print download progress, and capturing it makes a long download look + # hung. The npm-deprecation noise comes from the desktop build (captured + # to update.log), not this step. result = _m()._run_npm_install_deterministic( npm, _m().PROJECT_ROOT, @@ -4489,12 +4126,10 @@ def _update_node_dependencies() -> list[str]: def _log_only_write(text: str) -> None: """Write ``text`` to ``~/.hermes/logs/update.log`` only, never the terminal. - During ``hermes update`` ``sys.stdout`` is an ``_UpdateOutputStream`` that - mirrors to both the terminal and ``update.log``. Loud, low-signal - subprocess output (npm installs, the Electron/vite build, the cua-driver - installer's "Next steps" wall) should be captured and tucked into the log - so failures stay debuggable, without flooding the user's terminal. This - reaches past the mirroring stream straight to the underlying log handle. + During ``hermes update`` ``sys.stdout`` is an ``_UpdateOutputStream`` + mirroring to terminal and log; this reaches past it to the log handle so + loud, low-signal subprocess output (npm, Electron/vite, cua-driver "Next + steps") stays debuggable without flooding the terminal. """ if not text: return @@ -4531,13 +4166,11 @@ def _run_logged_subprocess(cmd, *, cwd=None, env=None): def _classify_fetch_failure(stderr: str) -> str: """Map git-fetch stderr to a one-line, user-facing diagnosis. - Order matters: curl surfaces HTTP failures as - ``fatal: unable to access '': The requested URL returned error: 429``, - so the rate-limit/outage checks must run BEFORE the generic - "unable to access" network check or a GitHub 429/5xx gets misreported as a - local network problem. The caller always prints the first raw stderr line - alongside this diagnosis — the friendly message adds guidance, it never - replaces the wire error. + Order matters: curl reports HTTP failures as ``unable to access '': + The requested URL returned error: 429``, so the rate-limit/outage checks + must run BEFORE the generic "unable to access" network check. The caller + always prints the first raw stderr line too — this adds guidance, it + never replaces the wire error. """ def _has_http_code(*codes: str) -> bool: @@ -4615,10 +4248,8 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if sys.platform == "win32": git_cmd = ["git", "-c", "windows.appendAtomically=false"] - # A crashed/interrupted fetch can leave .git/shallow.lock (or another git - # lock file) behind; every later fetch then fails with "File exists" and - # the check reports a hard failure (or, in the banner path, silently - # compares stale refs). Self-heal abandoned locks before fetching. + # An interrupted fetch can leave .git/shallow.lock (or another lock) behind, + # making every later fetch fail with "File exists". Self-heal before fetching. from hermes_cli.gitlock import clear_stale_git_locks, clear_stale_tmp_packs cleared = clear_stale_git_locks(_m().PROJECT_ROOT) @@ -4631,111 +4262,59 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if swept: print(f" (removed {len(swept)} aborted-fetch pack temp file(s))") - # Fetch only the branch we compare against; prefer upstream as the canonical - # reference. A bare `git fetch ` pulls every ref, and this repo has - # thousands of auto-generated branches, so scope the fetch to . - # Note: upstream/ may not exist for non-main branches (a fork's - # bb/gui has no upstream counterpart), so when the caller picks a - # non-default branch we skip the upstream probe and use origin directly. - # Installer checkouts are shallow (`git clone --depth 1`). A plain - # `git fetch` would unshallow the repo (dragging in the whole history — - # the exact cost the shallow clone avoided) and the rev-list count below - # would then report a huge bogus "behind" number. Detect shallow up front: - # fetch with --depth 1 to preserve the boundary and report presence-only. + # Fetch only : a bare `git fetch ` pulls thousands of + # auto-generated branches. Prefer upstream as canonical, but only for main + # (a fork's non-default branch has no upstream counterpart). Installer + # checkouts are shallow (`--depth 1`); a plain fetch would unshallow them + # and rev-list would report a huge bogus "behind" count, so fetch with + # --depth 1 and report presence-only. is_shallow = ( - subprocess.run( - git_cmd + ["rev-parse", "--is-shallow-repository"], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ).stdout.strip() + _git_run(git_cmd, ["rev-parse", "--is-shallow-repository"]).stdout.strip() == "true" ) depth_args = ["--depth", "1"] if is_shallow else [] if branch == "main": - # Probe locally (~6 ms) whether an 'upstream' remote exists at all - # before spending a network fetch on it. Non-fork installs have no - # 'upstream' remote, and the old flow burned a failed network attempt - # (~0.3-1 s) on every --check before falling back to origin. + # Probe locally (~6 ms) for an 'upstream' remote before spending a + # network fetch (~0.3-1 s) that non-fork installs would always fail. has_upstream_remote = ( - subprocess.run( - git_cmd + ["remote", "get-url", "upstream"], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ).returncode + _git_run(git_cmd, ["remote", "get-url", "upstream"]).returncode == 0 ) fetch_result = None if has_upstream_remote: print("→ Fetching from upstream...") - fetch_result = subprocess.run( - git_cmd + ["fetch"] + depth_args + ["upstream", branch], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - **_no_prompt_git_kwargs(), - ) + fetch_result = _git_run(git_cmd, ["fetch"] + depth_args + ["upstream", branch], network=True) if fetch_result is not None and fetch_result.returncode == 0: - upstream_exists = True compare_branch = f"upstream/{branch}" else: # No upstream remote, or the upstream fetch failed — use origin. print("→ Fetching from origin...") - fetch_result = subprocess.run( - git_cmd + ["fetch"] + depth_args + ["origin", branch], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - **_no_prompt_git_kwargs(), - ) - upstream_exists = False + fetch_result = _git_run(git_cmd, ["fetch"] + depth_args + ["origin", branch], network=True) compare_branch = f"origin/{branch}" else: # Non-default branch: compare against origin/ directly. print("→ Fetching from origin...") - fetch_result = subprocess.run( - git_cmd + ["fetch"] + depth_args + ["origin", branch], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - **_no_prompt_git_kwargs(), - ) - upstream_exists = False + fetch_result = _git_run(git_cmd, ["fetch"] + depth_args + ["origin", branch], network=True) compare_branch = f"origin/{branch}" if fetch_result.returncode != 0: _print_fetch_failure(fetch_result.stderr) sys.exit(1) - # Verify the compare ref actually exists before asking rev-list about it. - # Without this, `git rev-list HEAD..origin/ --count` exits 128 and - # (with check=True) raises CalledProcessError, surfacing a Python - # traceback. Friendlier to detect-and-report. - verify_result = subprocess.run( - git_cmd + ["rev-parse", "--verify", "--quiet", compare_branch], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - ) + # Verify the compare ref exists first: rev-list on a bogus ref exits 128 + # and (with check=True) would surface a Python traceback. + verify_result = _git_run(git_cmd, ["rev-parse", "--verify", "--quiet", compare_branch]) if verify_result.returncode != 0: print(f"✗ Branch '{branch}' not found on {compare_branch.split('/', 1)[0]}.") sys.exit(1) if is_shallow: - # No history to count across the shallow boundary. Compare tip SHAs - # (mirrors the banner's _check_via_local_git), then try to recover the - # exact count via the GitHub compare API — the remote graph is complete - # even when the local one is truncated. - head_sha = subprocess.run( - git_cmd + ["rev-parse", "HEAD"], - cwd=_m().PROJECT_ROOT, capture_output=True, text=True, encoding="utf-8", errors="replace", - ).stdout.strip() - target_sha = subprocess.run( - git_cmd + ["rev-parse", compare_branch], - cwd=_m().PROJECT_ROOT, capture_output=True, text=True, encoding="utf-8", errors="replace", - ).stdout.strip() + # No history across the shallow boundary: compare tip SHAs (like the + # banner's _check_via_local_git), then recover the exact count via the + # GitHub compare API, whose graph is complete. + head_sha = _git_run(git_cmd, ["rev-parse", "HEAD"]).stdout.strip() + target_sha = _git_run(git_cmd, ["rev-parse", compare_branch]).stdout.strip() if head_sha and target_sha and head_sha == target_sha: print("✓ Already up to date.") else: @@ -4755,13 +4334,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): print(f" Run '{recommended_update_command()}' to install.") return - rev_result = subprocess.run( - git_cmd + ["rev-list", f"HEAD..{compare_branch}", "--count"], - cwd=_m().PROJECT_ROOT, - capture_output=True, - text=True, encoding="utf-8", errors="replace", - check=True, - ) + rev_result = _git_run(git_cmd, ["rev-list", f"HEAD..{compare_branch}", "--count"], check=True) behind = int(rev_result.stdout.strip()) if behind == 0: @@ -4776,18 +4349,14 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): def _ensure_fhs_path_guard() -> None: """Ensure /usr/local/bin is on PATH for RHEL-family root non-login shells. - Mirrors the post-symlink probe added to ``scripts/install.sh`` so that - existing FHS-layout root installs on RHEL/CentOS/Rocky/Alma 8+ get - repaired on ``hermes update`` without requiring a reinstall. The - installer's assumption that ``/usr/local/bin`` is on PATH for every - standard shell breaks on those distros in non-login interactive shells - (su, sudo -s, tmux panes, some web terminals): /etc/bashrc doesn't - add /usr/local/bin and /root/.bash_profile doesn't either. Symptom: - ``hermes`` prints ``command not found`` even though the symlink lives - at /usr/local/bin/hermes. + Mirrors the post-symlink probe in ``scripts/install.sh`` so existing FHS + root installs on RHEL/CentOS/Rocky/Alma 8+ get repaired on ``hermes + update``. In non-login interactive shells there (su, sudo -s, tmux panes) + neither /etc/bashrc nor /root/.bash_profile adds /usr/local/bin, so + ``hermes`` prints ``command not found`` despite the symlink. - Silent no-op on: non-Linux, non-root, non-FHS installs, and any system - where ``bash -i -c 'command -v hermes'`` already resolves. Idempotent. + Silent no-op on non-Linux, non-root, non-FHS installs, and wherever + ``bash -i -c 'command -v hermes'`` already resolves. Idempotent. """ if _m().sys.platform != "linux": return @@ -4864,26 +4433,17 @@ def _ensure_fhs_path_guard() -> None: def _ensure_acp_launcher() -> None: r"""Self-heal: install a ``hermes-acp`` launcher next to the ``hermes`` one. - Mirrors the launcher block in ``scripts/install.sh`` so existing installs - gain the ACP command on ``hermes update`` without a reinstall. ACP hosts - (Zed, JetBrains, Buzz Desktop) spawn the agent by resolving the - ``hermes-acp`` command name against the login-shell PATH; the console - script of that name lives inside the install's venv, which is not on that - PATH, so those hosts report Hermes as not installed even when it is. + Mirrors the launcher block in ``scripts/install.sh``. ACP hosts (Zed, + JetBrains, Buzz Desktop) resolve ``hermes-acp`` on the login-shell PATH, + but the console script lives inside the venv, so they report Hermes as + not installed. The shim just delegates to the sibling ``hermes`` launcher + with the ``acp`` subcommand, which is correct for every install layout. - The shim simply delegates to the sibling ``hermes`` launcher with the - ``acp`` subcommand, which makes it correct for every install layout - (venv wrapper, FHS symlink, pipx/pip console script) without having to - reconstruct interpreter/entrypoint paths. - - No-op on Windows (install.ps1 stages the ``hermes`` / ``hermes-acp`` - launchers into the managed binary dir ``$HermesHome\bin`` and puts THAT - on the user PATH — never the whole ``venv\Scripts`` dir, which would - shadow the user's ``python`` (#83797); when those launchers go missing, - ``hermes_cli._install_repair.ensure_windows_bin_launchers`` re-stages - them) and wherever a ``hermes-acp`` is already present next to the - ``hermes`` command. Unwritable directories (e.g. ``/usr/local/bin`` as - non-root) are skipped silently. Idempotent. + No-op on Windows: install.ps1 stages launchers into ``$HermesHome\bin`` + and puts THAT on PATH — never ``venv\Scripts``, which would shadow the + user's ``python`` (#83797); ``ensure_windows_bin_launchers`` re-stages + them. Also no-op where ``hermes-acp`` already exists next to ``hermes``. + Unwritable dirs (``/usr/local/bin`` as non-root) are skipped. Idempotent. """ if _m().sys.platform == "win32": # Windows launcher staging/repair lives in _install_repair @@ -4896,10 +4456,9 @@ def _ensure_acp_launcher() -> None: try: if not (hermes_cmd.is_file() or hermes_cmd.is_symlink()): continue - # Already present — a console script (pip/pipx install), an - # earlier shim, or a symlink. is_symlink() catches broken - # symlinks that exists() would miss; never follow-and-overwrite - # (the #21454 failure mode). + # Already present (console script, earlier shim, or symlink). + # is_symlink() catches broken symlinks exists() misses; never + # follow-and-overwrite (#21454). if acp_cmd.exists() or acp_cmd.is_symlink(): continue shim = ( @@ -4916,28 +4475,23 @@ def _ensure_acp_launcher() -> None: print(f" ✓ Installed hermes-acp launcher → {acp_cmd}") _PRE_UPDATE_SNAPSHOT_KEEP = 1 -# Sibling-profile snapshot ids from the current run's pre-update backup -# ({profile: snapshot_id}) — consumed by the post-update per-profile -# cron-jobs safety net (#66140). Module-level because the snapshot and the -# restore run in the same process but far apart in _cmd_update_impl. +# {profile: snapshot_id} from this run's pre-update backup, consumed by the +# post-update per-profile cron-jobs safety net (#66140). Module-level because +# snapshot and restore run far apart in _cmd_update_impl. _LAST_SIBLING_SNAPSHOTS: dict = {} -# Per-file size cap for the pre-update quick snapshot. Anything larger is -# skipped with a warning: the snapshot exists to protect small, hard-to- -# regenerate state (pairing JSONs, cron jobs, config, auth) — not to copy a -# multi-GB state.db on every update (observed: a 24 GB state.db added ~60s -# of wall time and silently ate 24 GB of disk per update). +# Per-file cap for the quick snapshot; larger files are skipped with a warning. +# The snapshot protects small, hard-to-regenerate state (pairing JSONs, cron, +# config, auth) — not a multi-GB state.db (a 24 GB one cost ~60s and 24 GB/update). _PRE_UPDATE_SNAPSHOT_MAX_FILE_SIZE = 1 << 30 # 1 GiB def _resolve_pre_update_backup_mode(args) -> str: """Resolve the pre-update backup mode: ``"off"``, ``"quick"``, or ``"full"``. - CLI flags win over config; ``--no-backup`` beats ``--backup`` when both - are set. Config accepts the mode strings plus legacy booleans: - ``true`` → ``full`` (the old zip behavior), ``false`` → ``off`` - (an explicit opt-out now disables the quick snapshot too — previously - it ran unconditionally, ignoring the user's setting). A missing key - defaults to ``quick``. + CLI flags win over config; ``--no-backup`` beats ``--backup``. Config + accepts the mode strings plus legacy booleans: ``true`` → ``full``, + ``false`` → ``off`` (an explicit opt-out also disables the quick + snapshot). Missing key defaults to ``quick``. """ if getattr(args, "no_backup", False): return "off" @@ -4976,24 +4530,19 @@ def _resolve_pre_update_backup_mode(args) -> str: def _run_pre_update_backup(args) -> Optional[str]: """Run the pre-update safety backup and return the quick-snapshot id. - Single consolidated mechanism gated on ``updates.pre_update_backup``: + Gated on ``updates.pre_update_backup``: - - ``off`` — nothing runs. Explicit user opt-out is honored fully. - - ``quick`` (default) — a state snapshot of critical small files - (pairing JSONs, cron jobs, config, auth; see ``_QUICK_STATE_FILES``) - under ``state-snapshots/``. Files over 1 GiB are skipped with a - warning so a bloated state.db can never stall the update - (issues #15733, #34600 are the reason this safety net exists). - - ``full`` — the quick snapshot PLUS a full zip of HERMES_HOME under - ``backups/`` (restorable via ``hermes import``; the #48200 wrong-path - wipe is the reason this level exists). + - ``off`` — nothing runs; explicit opt-out is honored fully. + - ``quick`` (default) — snapshot of critical small files + (``_QUICK_STATE_FILES``) under ``state-snapshots/``; files over 1 GiB + are skipped so a bloated state.db can never stall the update (#15733, + #34600). + - ``full`` — quick snapshot PLUS a zip of HERMES_HOME under ``backups/`` + (restorable via ``hermes import``; exists because of the #48200 wipe). - ``--backup`` forces ``full`` for one run; ``--no-backup`` forces ``off``. - Never raises — a backup failure should not block the update itself. - - Returns the quick-snapshot id (used by the post-update cron-jobs - restore safety net), or ``None`` when mode is ``off`` or the snapshot - failed. + ``--backup`` forces ``full``; ``--no-backup`` forces ``off``. Never raises. + Returns the quick-snapshot id (used by the post-update cron-jobs restore), + or ``None`` when mode is ``off`` or the snapshot failed. """ mode = _resolve_pre_update_backup_mode(args) @@ -5024,12 +4573,10 @@ def _run_pre_update_backup(args) -> Optional[str]: max_file_size=_PRE_UPDATE_SNAPSHOT_MAX_FILE_SIZE, ) - # After the snapshot, verify the source state.db is still intact. - # The snapshot was taken via _safe_copy_db (read-only SQLite backup - # API), but a concurrent process (antivirus, force-killed gateway - # releasing file handles, Windows filter driver) can corrupt the live - # file at any point. A silent zeroing at this point would proceed with - # the update and exit code 0 — exactly the #68474 symptom. + # Verify the live state.db is still intact after the snapshot: a + # concurrent process (antivirus, force-killed gateway, Windows filter + # driver) can corrupt it at any point, and a silent zeroing would + # otherwise proceed to exit 0 — the #68474 symptom. if snapshot_id: _src_path = _get_home() / "state.db" if _src_path.exists(): @@ -5086,18 +4633,13 @@ def _run_pre_update_backup(args) -> Optional[str]: f"◆ Sibling profile snapshot(s): " + ", ".join(sorted(_sibling_snaps)) ) - try: - from hermes_cli.update_receipt import record_step - - record_step( - "sibling_profile_snapshots", - True, - ", ".join( - f"{k}={v}" for k, v in sorted(_sibling_snaps.items()) - ), - ) - except Exception: - pass + _record_update_step( + "sibling_profile_snapshots", + True, + ", ".join( + f"{k}={v}" for k, v in sorted(_sibling_snaps.items()) + ), + ) global _LAST_SIBLING_SNAPSHOTS _LAST_SIBLING_SNAPSHOTS = _sibling_snaps except Exception as _sib_exc: @@ -5231,15 +4773,11 @@ def _wait_for_windows_update_gateway_exit( def _venv_core_imports_healthy() -> tuple[bool, str]: """Probe the project venv for the core imports the backend needs to boot. - Runs a tiny import check inside the venv interpreter (NOT this process — - ``hermes update`` may be driven by a different Python). Catches the - half-updated-venv state: git checkout current but a dependency sync that - failed or was killed partway (e.g. Windows access-denied on a loaded - .pyd), leaving imports like ``fastapi``'s new transitive deps missing. - Without this probe, ``hermes update`` on a current checkout prints - "Already up to date!" and returns without ever re-syncing dependencies — - the user's install stays broken no matter how many times they update - (ryanc's incident, July 2026). + Runs inside the venv interpreter (NOT this process — ``hermes update`` may + run under a different Python). Catches a half-updated venv: checkout + current but a dependency sync failed or was killed partway (e.g. Windows + access-denied on a loaded .pyd). Without it, a current checkout prints + "Already up to date!" and never re-syncs, so the install stays broken. Returns ``(healthy, detail)``. Never raises; unknown states report healthy so a probe failure can't force needless reinstalls. @@ -5247,14 +4785,11 @@ def _venv_core_imports_healthy() -> tuple[bool, str]: venv_dir = _m().PROJECT_ROOT / "venv" venv_python = venv_python_path(venv_dir, windows=_m()._is_windows()) if not venv_python.exists(): - # No venv interpreter at all. In a dev checkout that's normal (the - # dev may run hermes from any interpreter), so report healthy to - # avoid forcing reinstalls. But on a MANAGED install (the Windows - # installer / desktop bootstrap stamps `.hermes-bootstrap-complete`, - # and an interrupted update leaves `.update-incomplete`), the venv - # IS the install — its absence means a repair got interrupted after - # the old venv was moved aside, and "Already up to date!" would - # gaslight the user while nothing can run. + # No venv interpreter. Normal for a dev checkout (report healthy to + # avoid forced reinstalls), but on a MANAGED install (bootstrap stamp + # or `.update-incomplete` present) the venv IS the install — its + # absence means a repair was interrupted after the old venv was moved + # aside, and "Already up to date!" would be a lie. managed_markers = ( _m().PROJECT_ROOT / ".hermes-bootstrap-complete", _m()._update_marker_path(), @@ -5296,24 +4831,51 @@ def _venv_core_imports_healthy() -> tuple[bool, str]: return False, "; ".join(missing[:4]) return True, "" +def _self_and_non_gateway_ancestor_pids(psutil) -> set[int]: + """PIDs a venv-holder scan must never nominate: this process and its ancestry. + + #87594: do NOT blanket-exclude ancestors. When ``/update`` runs from a + messaging platform the updater is a CHILD of the gateway; hiding it means + the pause machinery never sees the one process it exists to stop and the + update dead-ends on ``venv-blocked``. Keep a GATEWAY ancestor visible (the + pause path stops it gracefully; a detached child survives on Windows) and + exclude every other ancestor — an updater must never nominate its own + interactive ancestry as a blocker. + """ + try: + from gateway.status import looks_like_gateway_command_line as _is_gw + except Exception: + _is_gw = None + skip: set[int] = {os.getpid()} + try: + for anc in psutil.Process().parents(): + try: + anc_cmdline = " ".join(anc.cmdline() or []) + except Exception: + anc_cmdline = "" + if _is_gw is not None and anc_cmdline and _is_gw(anc_cmdline): + continue + skip.add(int(anc.pid)) + except Exception: + pass + return skip + + def _detect_venv_python_processes( *, exclude_pids: set[int] | None = None ) -> list[tuple[int, str, str]]: """Find live processes running from the project venv's interpreter. - The hermes.exe shim guard misses the biggest lock-holder class on - Windows: the Desktop app's backend (``python.exe -m hermes_cli.main - serve``) and anything else running straight off ``venv\\Scripts\\python - (w).exe``. Those processes keep native ``.pyd`` extensions mapped, so a - dependency sync mid-update dies with access-denied and strands the venv - half-updated (ryanc's brotlicffi/_sodium.pyd incidents, July 2026). + The hermes.exe shim guard misses the biggest Windows lock-holder class: + the Desktop backend (``python.exe -m hermes_cli.main serve``) and anything + running off ``venv\\Scripts\\python(w).exe``. They keep native ``.pyd`` + files mapped, so a mid-update dependency sync dies with access-denied and + strands the venv half-updated. - Killing them from here is pointless — the Desktop app supervises its - backend and respawns it within seconds — so the caller should refuse and - tell the user to close the app instead. Returns ``(pid, name, cmdline)`` - tuples; empty off-Windows / without psutil / when nothing matches. The - calling process and its ancestors are always excluded (a CLI ``hermes - update`` itself runs from the venv python). Never raises. + Killing them is pointless (the Desktop app respawns its backend), so the + caller should refuse and ask the user to close the app. Returns + ``(pid, name, cmdline)`` tuples; empty off-Windows / without psutil / no + matches. This process and its ancestors are excluded. Never raises. """ if not _m()._is_windows(): return [] @@ -5333,40 +4895,14 @@ def _detect_venv_python_processes( root_prefix = str(_m().PROJECT_ROOT).lower().rstrip(os.sep) + os.sep skip: set[int] = set(exclude_pids or set()) - skip.add(os.getpid()) - try: - from gateway.status import looks_like_gateway_command_line as _is_gw - except Exception: - _is_gw = None - try: - for anc in psutil.Process().parents(): - # #87594: do NOT blanket-exclude ancestors. When `/update` runs - # from a messaging platform the updater is a CHILD of the gateway - # — excluding all ancestors hides the gateway from the scan, so - # the pause machinery downstream never sees the one process it - # exists to stop, and the update dead-ends on `venv-blocked`. - # A GATEWAY ancestor stays visible (the pause path stops it - # gracefully; a detached child updater survives its parent's - # stop on Windows). Every other ancestor (shells, terminals, - # this CLI's own venv python chain) stays excluded — an updater - # must never nominate its own interactive ancestry as blockers. - try: - anc_cmdline = " ".join(anc.cmdline() or []) - except Exception: - anc_cmdline = "" - if _is_gw is not None and anc_cmdline and _is_gw(anc_cmdline): - continue - skip.add(int(anc.pid)) - except Exception: - pass + skip |= _self_and_non_gateway_ancestor_pids(psutil) matches: list[tuple[int, str, str]] = [] try: - # On Windows, prefetching cmdline and cwd performs two expensive - # per-process queries. A busy workstation can have 500+ processes, so - # querying those fields for every unrelated process can exceed the - # Desktop preflight watchdog. First collect only cheap identity fields; - # fetch cmdline/cwd lazily for plausible Python/uv/Hermes candidates. + # On Windows cmdline/cwd are expensive per-process queries; with 500+ + # processes prefetching them can exceed the Desktop preflight watchdog. + # Collect cheap identity fields first, fetch cmdline/cwd lazily for + # plausible Python/uv/Hermes candidates. proc_iter = psutil.process_iter(["pid", "exe", "name"]) except Exception: return [] @@ -5401,10 +4937,9 @@ def _detect_venv_python_processes( cmdline_raw = "" cmdline_low = cmdline_raw.lower() # Fallback: uv/base-interpreter trampolines run a python whose exe is - # OUTSIDE the venv but which still imports from it and holds its .pyd - # files. Catch those by what they're running: a cmdline that references - # this venv's path, or a `-m hermes_cli.main ...` invocation tied to - # this install (install root in the cmdline or as the working dir). + # OUTSIDE the venv yet still holds its .pyd files. Match on the cmdline + # instead: this venv's path, or `-m hermes_cli.main` tied to this + # install (root in the cmdline or as cwd). if not is_holder and venv_prefix in cmdline_low: is_holder = True if not is_holder and "hermes_cli.main" in cmdline_low: @@ -5417,42 +4952,31 @@ def _detect_venv_python_processes( if not is_holder: continue name = info.get("name") or Path(exe).name - # Return the FULL cmdline: callers match against it (the Desktop - # preflight's pausable-gateway exemption parses for `gateway run`). - # Truncating here cut long managed-runtime interpreter paths before - # the `-m hermes_cli.main gateway run` argv, so autostarted gateways - # were misreported as blockers and the update dead-ended. Truncate - # only at display time. + # Return the FULL cmdline: callers parse it (the Desktop preflight's + # pausable-gateway exemption looks for `gateway run`). Truncating here + # once cut long interpreter paths before the argv, so autostarted + # gateways were misreported as blockers. Truncate only at display time. matches.append((int(pid), str(name), cmdline_raw)) return matches -# Native-extension modules that pin files inside the venv once imported. If -# the updater process itself has any of these loaded, the dependency sync -# below cannot rewrite the backing ``.pyd``/``.dll`` — Windows blocks REPLACE -# on a mapped image — and the update dies with ``os error 5`` between -# uninstall and reinstall, stranding the venv half-updated (#83569). -# ``cryptography`` is the canonical case: ``hermes_cli.main`` used to import -# it at startup while resolving external secret sources; ``PyYAML``'s -# ``_yaml`` C extension is loaded by every CLI process (config parsing). -# Keep this guard as defence-in-depth against future eager imports (new -# secret sources, plugins absorbed into core, refactors of the startup -# order) — but the guard must be HONEST (#86735/#86780/#86781: a preflight -# that fired on every run, before the fetch, re-bricked the exact flow it -# was meant to protect). Two honesty gates: +# Native-extension modules that pin files inside the venv once imported. If +# the updater itself has one loaded, Windows blocks REPLACE on the mapped +# ``.pyd``/``.dll`` and the sync dies with ``os error 5`` between uninstall +# and reinstall, stranding the venv half-updated (#83569). ``cryptography`` +# is the canonical case; PyYAML's ``_yaml`` is loaded by every CLI process. +# Kept as defence-in-depth against future eager imports, but the guard must +# be HONEST (#86735/#86780/#86781: a preflight firing on every run, before +# the fetch, re-bricked the flow it protected). Two honesty gates: # -# 1. It only fires when the dependency sync would actually REWRITE the -# loaded distribution (``_dependency_sync_would_rewrite``): if the -# installed version already satisfies the on-disk pyproject pins, uv/pip -# will not touch the mapped ``.pyd``, so there is no lock to trip. -# 2. It runs AFTER the code swap (git pull / ZIP commit), immediately -# before the venv rewrite — so the on-disk pyproject is the NEW one -# (gate 1 compares against the right target) and a deferral no longer -# strands the user on the old checkout: the next launch's marker -# recovery completes the dependency install against the already-updated -# pyproject. +# 1. Fire only when the sync would actually REWRITE the loaded distribution +# (``_dependency_sync_would_rewrite``); a satisfied pin means uv/pip +# never touch the mapped ``.pyd``. +# 2. Run AFTER the code swap, right before the venv rewrite — so gate 1 +# compares against the NEW pyproject and a deferral leaves the user on +# new code with only the dependency install pending for the next launch's +# marker recovery. # -# Keys are module prefixes in ``sys.modules``; values are -# ``(display name, PyPI distribution name)``. +# Keys are ``sys.modules`` prefixes; values are ``(display name, PyPI dist)``. _SELF_LOCKING_NATIVE_MODULES: dict[str, tuple[str, str]] = { "cryptography.hazmat.bindings._rust": ("cryptography (_rust.pyd)", "cryptography"), "yaml._yaml": ("PyYAML (_yaml.pyd)", "pyyaml"), @@ -5462,19 +4986,14 @@ _SELF_LOCKING_NATIVE_MODULES: dict[str, tuple[str, str]] = { def _dependency_sync_would_rewrite(dist_name: str) -> bool | None: """Whether ``uv pip install -e .[all]`` would replace *dist_name*'s files. - Compares the installed distribution version against every applicable - requirement for it in the on-disk ``pyproject.toml`` (base dependencies - plus all optional extras). Returns: + Compares the installed version against every applicable requirement in + the on-disk ``pyproject.toml`` (base deps plus all extras). ``False`` — + every pin satisfied, a mapped extension is NOT at risk; ``True`` — some + pin unsatisfied or dist missing; ``None`` — undeterminable. - - ``False`` — installed version satisfies every pin: the resolver will - leave the wheel alone, so a mapped extension is NOT at risk. - - ``True`` — some pin is not satisfied (or the distribution is - missing): the sync will rewrite it. - - ``None`` — could not determine (parse failure, unparseable pins). - - Never raises. Callers treat ``None`` as fail-OPEN (no deferral): a - module in the registry can be loaded by every process (PyYAML), so - deferring on uncertainty would recreate the #86735 always-firing loop. + Never raises. Callers treat ``None`` as fail-OPEN (no deferral): PyYAML + is loaded by every process, so deferring on uncertainty would recreate + the #86735 always-firing loop. """ try: from importlib import metadata as _ilmd @@ -5537,14 +5056,12 @@ def _detect_self_loaded_native_modules() -> list[str]: for prefix, (display, dist) in _SELF_LOCKING_NATIVE_MODULES.items(): if prefix not in sys.modules: continue - # Defer ONLY on a CONFIRMED pending rewrite. An "unknown" result - # (unreadable/unparseable pyproject, no pin found) must fail OPEN: - # PyYAML is loaded in every CLI process, so treating unknown as - # at-risk would re-create the exact always-firing loop this guard's - # first version caused (#86735). The downside of a missed deferral - # is the pre-existing failure mode — a mid-sync os error 5 that the - # marker recovery already handles — which is strictly less harmful - # than an update that can never run. + # Defer ONLY on a CONFIRMED pending rewrite; "unknown" must fail OPEN, + # since PyYAML is loaded in every CLI process and treating unknown as + # at-risk recreated the always-firing loop (#86735). A missed deferral + # only yields the pre-existing mid-sync os error 5, which marker + # recovery already handles — far less harmful than an update that + # can never run. if _m()._dependency_sync_would_rewrite(dist) is not True: continue found.append(display) @@ -5554,22 +5071,15 @@ def _detect_self_loaded_native_modules() -> list[str]: def _abort_dependency_sync_if_self_locked(gateway_resume=None) -> None: """Defer the venv rewrite when THIS process holds something it must replace. - Runs at the last moment before the venv rewrite — after the code swap — - so the on-disk pyproject reflects the update target and a deferral + Runs after the code swap, right before the venv rewrite, so a deferral leaves the user on NEW code with only the dependency install pending. - No-op when nothing at-risk is held. + No-op when nothing at-risk is held. Two hazards with different recoveries: - Two hazards, both "this process holds a file the sync must replace", and - they end differently because their recoveries differ: - - - A mapped native extension (``.pyd``). Exit 2 and let the next launch's - marker recovery finish the install: that launch runs the install before - importing anything heavy, so it maps nothing and the swap succeeds. - - - The ``hermes.exe`` console shim we were launched from (#88838, #89599). - The marker cannot help here — every future ``hermes`` launch is also the - shim, so deferring to the next launch defers forever. Hand the install - to a child under the venv interpreter and exit, releasing the shim. + - A mapped native extension (``.pyd``): exit 2 and let the next launch's + marker recovery finish the install before importing anything heavy. + - The ``hermes.exe`` shim we were launched from (#88838, #89599): every + future launch is also the shim, so the marker would defer forever. + Hand the install to a child under the venv interpreter and exit. """ locked = _m()._detect_self_loaded_native_modules() if locked: @@ -5620,16 +5130,13 @@ _holder_value_flags_cache: frozenset | None = None def _holder_value_flags() -> frozenset: """Top-level CLI flags that consume a value — derived from the REAL parser. - Introspects ``build_top_level_parser()`` (every option with nargs != 0) - so the holder classifier can never drift from the argparse surface - (#91869 review: a handwritten subset misparsed ``--reasoning high - serve`` as subcommand ``high`` and ``-m dashboard serve`` as - ``dashboard`` — recreating the wrong-hint class). The pre-argparse - profile selectors (``--profile``/``-p``, ``--config``) are added - explicitly since they are stripped before argparse sees argv. Falls - back to a static snapshot when the parser cannot be imported (the - updater must classify holders even mid-upgrade on a broken tree). - Cached per process. + Introspects ``build_top_level_parser()`` (every option with nargs != 0) so + the holder classifier can't drift from argparse (#91869: a handwritten + subset misparsed ``--reasoning high serve`` as subcommand ``high``). The + pre-argparse profile selectors (``--profile``/``-p``, ``--config``) are + added explicitly since they're stripped before argparse sees argv. Falls + back to a static snapshot when the parser can't be imported (the updater + must classify holders even on a broken tree). Cached per process. """ global _holder_value_flags_cache if _holder_value_flags_cache is not None: @@ -5651,13 +5158,11 @@ def _holder_value_flags() -> frozenset: def _hermes_holder_subcommand(cmdline: str) -> str | None: """The actual Hermes SUBCOMMAND a venv-holder argv runs, or None. - Token-based, never substring (#90778: ``kanban --preserve-cache`` - contained \"serve\" and got labeled as the Desktop backend). Finds the - ``hermes_cli.main`` / ``hermes(.exe)`` entry token, then returns the - first following token that is not a flag or a flag's value. Profile - selectors (``--profile X``, ``-p X``) are skipped like the canonical - gateway matcher does. Returns None when no subcommand can be - determined — callers must NOT guess a label in that case. + Token-based, never substring (#90778: ``kanban --preserve-cache`` contains + \"serve\" and got labeled as the Desktop backend). Finds the + ``hermes_cli.main`` / ``hermes(.exe)`` entry token, then returns the first + following token that isn't a flag or a flag's value (profile selectors + skipped). None when undeterminable — callers must NOT guess a label. """ try: import shlex @@ -5734,26 +5239,17 @@ def _format_venv_python_holders_message(matches: list[tuple[int, str, str]]) -> def _venv_launcher_ancestors(pids: list[int]) -> list[int]: """Return venv-interpreter ancestors of *pids* that hold the install open. - On Windows a gateway started through the venv shim is a **two-process - chain**: ``venv\\Scripts\\python.exe`` (the launcher, which keeps native - ``.pyd`` files from the venv mapped) spawns the actual interpreter from - uv's managed CPython directory (``AppData\\Roaming\\uv\\python\\...``). - The gateway writes its PID file from the *child*, so - ``find_gateway_pids()`` — and therefore this module's pause set — only - ever sees the uv-side worker. + On Windows a gateway started through the venv shim is a two-process chain: + ``venv\\Scripts\\python.exe`` (the launcher, which keeps venv ``.pyd`` + files mapped) spawns the real interpreter from uv's managed CPython. The + PID file is written by the *child*, so ``find_gateway_pids()`` / the pause + set only see the uv-side worker, while ``_detect_venv_python_processes()`` + (venv path prefix) sees the *launcher*. The sets are disjoint, so a paused + gateway still tripped the venv-holder guard and aborted the update. - ``_detect_venv_python_processes()`` matches on the venv path prefix, so - the guard downstream of the pause sees the *launcher* instead. The two - sets are disjoint, which meant a paused gateway still tripped the - venv-holder guard and aborted the update every time (the Desktop - "venv-blocked: N process(es) hold the install" dead-end, where the - reported holder is a gateway the updater believes it already stopped). - - Walking one hop up from each mapped gateway PID and keeping ancestors - that live under the project venv closes the gap. Only the venv-side - parent is returned — unrelated ancestors (the Scheduled Task's - ``cmd.exe``, an operator's shell) are ignored so we never widen the - blast radius beyond the gateway's own launcher. Never raises. + Walk one hop up from each mapped gateway PID and keep only ancestors under + the project venv; unrelated ancestors (the Scheduled Task's ``cmd.exe``, + an operator's shell) are ignored to bound the blast radius. Never raises. """ if not _m()._is_windows() or not pids: return [] @@ -5768,28 +5264,7 @@ def _venv_launcher_ancestors(pids: list[int]) -> list[int]: except OSError: venv_prefix = str(venv_dir).lower().rstrip(os.sep) + os.sep - # Never return ourselves or our own ancestry: a CLI ``hermes update`` - # runs from the venv python and would otherwise nominate itself. - # Same #87594 carve-out as _detect_venv_python_processes: a GATEWAY - # ancestor is not "our own ancestry" in the interactive sense — it is - # the process the pause machinery must see (the /update-from-gateway - # topology makes the updater the gateway's child). - try: - from gateway.status import looks_like_gateway_command_line as _is_gw - except Exception: - _is_gw = None - skip: set[int] = {os.getpid()} - try: - for anc in psutil.Process().parents(): - try: - anc_cmdline = " ".join(anc.cmdline() or []) - except Exception: - anc_cmdline = "" - if _is_gw is not None and anc_cmdline and _is_gw(anc_cmdline): - continue - skip.add(int(anc.pid)) - except Exception: - pass + skip = _self_and_non_gateway_ancestor_pids(psutil) found: list[int] = [] for pid in pids: @@ -5816,25 +5291,20 @@ def _leftover_pausable_gateway_pids( ) -> list[int] | None: """PIDs from *matches* when every remaining venv holder is a pausable gateway. - ``_pause_windows_gateways_for_update()`` stops every gateway its discovery - finds, but the venv-holder guard downstream sees the process table as it - is *now*: a gateway respawned by its supervisor (Scheduled Task, login - watchdog) inside the pause→guard window, or one started through a spawn - path the discovery does not map, still holds venv ``.pyd`` files and - would dead-end the update — an abort pointed at exactly the kind of - process the pause machinery exists to stop. + ``_pause_windows_gateways_for_update()`` stops the gateways its discovery + finds, but the venv-holder guard sees the process table as it is *now*: a + gateway respawned by its supervisor inside the pause→guard window, or one + started through an unmapped spawn path, still holds venv ``.pyd`` files and + would dead-end the update on exactly the process the pause exists to stop. Holders are classified with the same matcher the Desktop preflight uses - to exempt them (``_is_pausable_gateway``), so the preflight's exemption - and this guard's tolerance cannot drift apart — matcher drift between - two views of the same process table is what produced the launcher/worker - dead-end fixed above. The scan captures only a 120-char cmdline prefix, - so the live argv is re-read where psutil allows; an unreadable argv - falls back to the captured prefix. + (``_is_pausable_gateway``) so exemption and tolerance cannot drift apart. + The scan keeps only a 120-char cmdline prefix, so live argv is re-read via + psutil when possible, falling back to the prefix. - Returns ``None`` when any holder is not a pausable gateway — an operator - REPL, a stray script, or the Desktop backend has no pause machinery - downstream, and the guard must keep refusing exactly as before. + Returns ``None`` when any holder is not a pausable gateway (operator REPL, + stray script, Desktop backend) — nothing downstream can pause it, so the + guard must keep refusing. """ from hermes_cli._scan_venv_blockers import _is_pausable_gateway @@ -5862,17 +5332,13 @@ def _refuse_gateway_ancestor_tree_kill( ) -> bool: """Refuse a plain Windows update that would kill its own process tree. - A chat agent can launch plain ``hermes update`` through its terminal tool. - In that topology the updater is a child of the gateway. The leftover - holder recovery below uses ``taskkill /T /F`` on Windows, so force-stopping - that gateway also kills the updater before it can mutate the checkout - (#98814). - - ``/update`` uses the supported ``--gateway`` hand-off and is deliberately - exempt: it detaches the updater and provides file-based progress/result - delivery. For every other invocation, refuse only when a nominated - gateway is positively identified as this process's ancestor. If ancestry - cannot be established, preserve the existing holder recovery behavior. + A chat agent can run plain ``hermes update`` via its terminal tool, making + the updater a child of the gateway; leftover-holder recovery uses + ``taskkill /T /F``, so force-stopping that gateway kills the updater before + it mutates the checkout (#98814). ``/update`` (``--gateway`` hand-off) is + exempt: it detaches the updater with file-based progress/result delivery. + Otherwise refuse only when a nominated gateway is positively an ancestor of + this process; if ancestry cannot be established, keep existing recovery. """ if gateway_mode or not pids: return False @@ -6001,16 +5467,11 @@ def _relaunch_stopped_serves(token: dict) -> None: " ⚠ Some stopped backends could not be relaunched automatically; " "restart them manually (hermes serve --host --port )." ) - try: - from hermes_cli.update_receipt import record_step - - record_step( - "serve_relaunch", - not failed and not skipped, - f"relaunched={len(commands) - len(failed)} failed={len(failed)} skipped={skipped}", - ) - except Exception: - pass + _record_update_step( + "serve_relaunch", + not failed and not skipped, + f"relaunched={len(commands) - len(failed)} failed={len(failed)} skipped={skipped}", + ) def _orphaned_desktop_backend_pids( @@ -6019,38 +5480,31 @@ def _orphaned_desktop_backend_pids( """PIDs from *matches* when every remaining holder is an ORPHANED backend. The venv-holder guard refuses on the Desktop app's ``serve`` backend by - design: while the Desktop is open, killing its backend is futile (the app - supervises and respawns it within seconds), so the user must close the - app. But in the GUI-updater handoff path the Desktop has *already - exited* — by contract it tree-kills its backends and waits for the venv - shim before spawning hermes-setup, and the update-in-progress marker - parks any relaunched Desktop from spawning a fresh backend (#50238). A - ``serve`` backend still holding the venv at that point is a straggler - whose supervisor is gone: SIGTERM raced its spawn, or it belongs to a - crashed window. Nothing will respawn it, and refusing on it dead-ends - the update with "Hermes is still running" while the user stares at zero - open windows (ryanc's 2026-08-09 01:59/02:17 failures). + design: while the Desktop is open, killing it is futile (the app respawns + it within seconds). But in the GUI-updater hand-off the Desktop has + *already exited* — by contract it tree-kills its backends before spawning + hermes-setup, and the update-in-progress marker parks any relaunched + Desktop (#50238). A ``serve`` backend still holding the venv then is a + straggler whose supervisor is gone (SIGTERM raced its spawn, or a crashed + window); refusing on it dead-ends the update with "Hermes is still + running" while the user sees zero open windows. A holder qualifies only when BOTH hold: - its cmdline is a Hermes backend (``hermes_cli.main`` + ``serve`` / ``dashboard``), and - its supervising parent is demonstrably gone: the parent PID no longer - exists, or the PID was reused (parent created *after* the child). + exists, or was reused (parent created *after* the child). - Tree-aware: the scanner can return an orphaned backend AND one of its - managed-runtime descendants (the ``.hermes-runtime`` interpreter child) - in the same holder set. That descendant has a live parent — the orphaned - backend itself — and isn't a ``serve`` cmdline, so per-process rules - would refuse a set that is entirely safe to reap. Holders that sit - inside an accepted orphan root's tree are therefore folded into that - root (only roots are returned; ``taskkill /T`` reaps the descendants). + Tree-aware: the scanner may also return an orphan's managed-runtime child + (the ``.hermes-runtime`` interpreter), which has a live parent and is not + a ``serve`` cmdline. Holders inside an accepted orphan root's tree are + folded into that root; only roots are returned (``taskkill /T`` reaps + descendants). - Any other live-parent backend (the Desktop is still open), non-backend - holder outside an orphan tree, or unprovable case disqualifies the whole - set — the guard must keep refusing exactly as before. Returns ``None`` - in that case, or when psutil is unavailable (can't prove orphanhood → - refuse). Never raises. + Any other live-parent backend, non-backend holder outside an orphan tree, + or unprovable case disqualifies the whole set → ``None`` (keep refusing). + Also ``None`` when psutil is unavailable. Never raises. """ try: import psutil # type: ignore @@ -6081,12 +5535,10 @@ def _orphaned_desktop_backend_pids( continue try: proc = psutil.Process(int(pid)) - # Fingerprint from the SAME psutil handle used for classification - # below, quantized to centiseconds — the exact scheme - # gateway.status.get_process_start_time uses on Windows, so the - # value round-trips through pid_is_hermes at kill time. (Reading - # /proc//stat here instead would consult the HOST process - # table and use different units.) + # Fingerprint from the SAME psutil handle, quantized to centiseconds + # like gateway.status.get_process_start_time on Windows, so it + # round-trips through pid_is_hermes at kill time (/proc//stat + # would read the HOST table in different units). process_start_time = int(round(proc.create_time() * 100)) except psutil.NoSuchProcess: # The candidate itself exited during classification; there is @@ -6102,12 +5554,9 @@ def _orphaned_desktop_backend_pids( # PID-reuse check: a "parent" created after its child is a # recycled PID, not the real (dead) supervisor. if parent.create_time() <= proc.create_time(): - # Live parent — NOT a root. But it may still be a - # descendant of an orphan root: the venv python.exe is - # a trampoline that re-execs the uv-managed interpreter - # with the SAME backend argv, so the worker half of the - # two-process chain lands here. Defer to pass 2 instead - # of refusing outright. + # Live parent — not a root, but possibly an orphan root's + # descendant (the venv python.exe trampoline re-execs the + # uv interpreter with the SAME argv). Defer to pass 2. remaining.append((int(pid), low)) continue except psutil.NoSuchProcess: @@ -6139,9 +5588,9 @@ def _ledger_reapable_backend_pids( ) -> list[int]: """PIDs positively identified by the spawn ledger as orphaned backends. - The strongest rung: instead of inferring lineage from PPIDs or cmdline - shape, look each venv holder up in the machine spawn ledger - (``hermes_cli.process_identity``). A holder qualifies when ALL of: + The strongest rung: look each venv holder up in the machine spawn ledger + (``hermes_cli.process_identity``) instead of inferring lineage from PPIDs + or cmdline shape. A holder qualifies when ALL of: - its ``(pid, create_time)`` matches a live ledger entry (PID reuse cannot forge this pair); @@ -6149,11 +5598,9 @@ def _ledger_reapable_backend_pids( gateway — never interactive processes); - the entry's recorded SPAWNER is provably dead (``spawner_is_dead``). - Unlike the heuristic rungs, this is safe in ANY update context — no - hand-off contract needed — because the ownership claim is explicit: the - process itself declared who supervises it, and that supervisor is gone. - Holders not in the ledger are simply not returned (they fall through to - the later rungs); they never disqualify the identified ones. Never raises. + Safe in ANY update context — the process itself declared its supervisor + and that supervisor is gone. Holders not in the ledger fall through to + later rungs and never disqualify identified ones. Never raises. """ try: from hermes_cli.process_identity import ( @@ -6184,41 +5631,27 @@ def _handoff_reapable_backend_pids( """PIDs of Hermes ``serve``/``dashboard`` backends safe to reap during a GUI-updater hand-off, INCLUDING ones with a still-live parent. - Complements ``_orphaned_desktop_backend_pids``, which only reaps backends - whose supervisor is provably dead. That check returns ``None`` (keep - refusing) the moment ANY holder still has a live parent — which is exactly - the case that produced the field incident this fixes: a Windows Desktop - update hand-off (``update --yes --gateway --force``) left a *swarm* of - per-profile ``serve`` backends (mr-tester, probe-inherit, turqoise, …) - holding ``cryptography\\_rust.pyd``. Several still had a lingering - parent (the tearing-down Electron process, or the two-hop venv - launcher→worker chain mid-exit), so the orphan check disqualified the - WHOLE set and the update dead-ended — the user saw a 12-minute hang, then - force-closed, and the half-done state stranded bot sessions. + Complements ``_orphaned_desktop_backend_pids``, which returns ``None`` + (keep refusing) the moment ANY holder has a live parent. That produced a + field incident: a Windows Desktop hand-off (``update --yes --gateway + --force``) left a swarm of per-profile ``serve`` backends holding + ``cryptography\\_rust.pyd``, several with a lingering parent (tearing-down + Electron, or the launcher→worker chain mid-exit), so the orphan check + disqualified the WHOLE set and the update hung for 12 minutes. - The hand-off is the safe signal: when the update-incomplete marker is - present (the GUI updater claimed it) AND this is a ``--gateway`` hand-off - run AND no live Desktop shim (``hermes.exe``) is open, NOTHING legitimate - is supervising or respawning a ``serve`` backend from this venv — by the - hand-off contract the Desktop tree-kills its backends and parks any - relaunch behind the marker (#50238). Any ``serve`` backend still holding - the venv here is therefore a leak, live parent or not, and reaping its - tree is correct rather than a race. + The hand-off is the safe signal: with the update-incomplete marker claimed + AND a ``--gateway`` run AND no live Desktop shim (``hermes.exe``), nothing + legitimate supervises or respawns a ``serve`` backend from this venv (the + Desktop tree-kills its backends and parks relaunch behind the marker, + #50238). Any ``serve`` backend still holding the venv is a leak, live + parent or not, and tree-reaping it is correct rather than a race. - Guarded conservatively: - - - Only Hermes backends (``hermes_cli.main`` + ``serve``/``dashboard``) - from THIS install's venv qualify; a non-backend holder (operator REPL, - stray script) disqualifies the whole set → ``None`` (keep refusing), so - we never widen the blast radius during a hand-off. - - Only runs when the CALLER has confirmed the hand-off context - (``args.gateway`` AND a claimed update-incomplete marker AND no live - ``hermes.exe`` shim) — outside that gate this function is never called - and the stricter orphan-only path stands. - - psutil unavailable → ``None`` (can't re-read argv to classify → refuse). - - Returns the backend root PIDs to tree-reap, or ``None`` to leave the - decision to the caller's existing rungs. Never raises. + Guards: only Hermes backends (``hermes_cli.main`` + ``serve``/``dashboard``) + qualify — a non-backend holder disqualifies the whole set → ``None``; the + CALLER must have confirmed the hand-off gate above (outside it the stricter + orphan-only path stands); psutil unavailable → ``None``. Returns backend + root PIDs to tree-reap, or ``None`` to leave the decision to the caller's + other rungs. Never raises. """ try: import psutil # type: ignore @@ -6299,15 +5732,12 @@ def _stop_process_trees( def _looks_like_desktop_control_plane(cmdline: str) -> bool: """True for this-install ``hermes serve`` / ``hermes dashboard`` argv. - That is the Desktop control plane, not the messaging gateway. Serve and - dashboard do not host platform adapters (#92091); do not feed this into - ``looks_like_gateway_command_line``. - - Token-based via the parser-derived subcommand classifier — never - substring (#90778/#91869: ``kanban --preserve-cache`` contains "serve", - ``-m dashboard chat`` contains " dashboard"; both are NOT control - planes). A cmdline whose subcommand cannot be determined is NOT a - control plane — callers must not guess ownership. + That is the Desktop control plane, not the messaging gateway (serve and + dashboard host no platform adapters, #92091); do not feed this into + ``looks_like_gateway_command_line``. Token-based via the parser-derived + subcommand classifier — never substring (#90778/#91869: ``kanban + --preserve-cache`` contains "serve", ``-m dashboard chat`` contains + " dashboard"). An undeterminable subcommand is NOT a control plane. """ if "hermes_cli.main" not in (cmdline or "").lower(): return False @@ -6470,9 +5900,8 @@ def _stop_windows_gateway_service( return _time.sleep(0.2) if service.status() == "stopped": - # We only return if the original processes have also exited their identity. - # A lingering process with a matching creation time means the venv mutation - # must not proceed — fail closed. + # Return only if the original processes are gone too; a lingering + # matching-identity process means venv mutation is unsafe — fail closed. alive_after_stop = [ pid for pid, create_time in expected_processes @@ -6484,8 +5913,7 @@ def _stop_windows_gateway_service( f"{alive_after_stop}" ) return - # If we reach here, the timeout elapsed without the service reaching a stable stopped state - # while its original descendants are still alive. Fail closed — venv mutation is unsafe. + # Timeout with the original descendants still alive — fail closed; venv mutation is unsafe. raise RuntimeError( f"Windows service {name} did not stop within {timeout:.0f}s; venv mutation unsafe." ) @@ -6593,21 +6021,17 @@ def _pause_windows_gateways_for_update() -> dict | None: f"Could not discover Windows gateway PIDs before update: {exc}" ) from exc if not running_pids: - # No gateway is running right now, but the user may have installed an - # autostart entry (Scheduled Task or Startup-folder login item) — that - # is an explicit "I want a gateway" signal. A gateway that died between - # updates (e.g. the spawning terminal/TUI closed, taking its child with - # it) would otherwise never come back: the autostart entry only fires on - # the next login, and the update flow's resume path only relaunched - # gateways that were running when the update began. Cold-start one after - # the update so an installed gateway is actually up post-update. Users - # who run gateway-less (no autostart entry) get nothing forced on them. + # No gateway is running, but an installed autostart entry (Scheduled + # Task / Startup-folder login item) is an explicit "I want a gateway" + # signal. A gateway that died between updates (e.g. its spawning + # terminal closed) would otherwise stay down until next login, since + # the resume path only relaunches gateways that were running. Cold-start + # one after the update; gateway-less users get nothing forced on them. # - # Exception: Desktop currently owns this install's gateway lifecycle - # (live supervised serve/dashboard). A vestigial Startup/Scheduled - # Task is not the owner — spawning ``gateway run`` beside Desktop - # races ports/state (#76129). Serve is the control plane, not proof - # messaging is served; the skip is ownership, not liveness (#92091). + # Exception: Desktop owns this install's gateway lifecycle (live + # supervised serve/dashboard); a vestigial autostart entry is not the + # owner, and spawning ``gateway run`` beside Desktop races ports/state + # (#76129). The skip is ownership, not liveness (#92091). try: if _desktop_owns_gateway_lifecycle(): logger.debug( @@ -6650,13 +6074,11 @@ def _pause_windows_gateways_for_update() -> dict | None: profiles[str(proc.profile)] = int(pid) mapped_pids.append(int(pid)) _write_update_planned_stop_marker(Path(proc.path), int(pid)) - # Socket-first pause (#92091 step 2): ask the gateway to drain and - # exit itself instead of relying on the marker poll + force-kill - # ladder. A positive ACK means the gateway is running its own - # graceful restart path (same drain as SIGUSR1/service restarts) and - # will release its venv handles on the way out. No answer (older - # gateway, no socket) → the marker watcher / force-kill fallback - # below behaves exactly as before this verb existed. + # Socket-first pause (#92091 step 2): ask the gateway to drain and exit + # itself. A positive ACK means it runs its own graceful restart path + # (same drain as SIGUSR1/service restarts) and releases venv handles on + # exit. No answer (older gateway, no socket) → the marker poll / + # force-kill ladder below behaves exactly as before. try: from gateway.control_socket import pause_gateway_for_update @@ -6668,18 +6090,15 @@ def _pause_windows_gateways_for_update() -> dict | None: "Socket pause unavailable for gateway %s: %s", pid, exc ) - # Resolve each mapped worker's venv-side launcher BEFORE draining: the - # drain stops tracking a PID exactly when it dies, so a gracefully - # drained worker is gone by the time the wait returns — and a dead pid's - # parent cannot be recovered (psutil raises NoSuchProcess). The snapshot - # is stopped after the drain alongside the survivors. + # Resolve each mapped worker's venv-side launcher BEFORE draining: a + # gracefully drained worker is gone when the wait returns, and a dead + # pid's parent cannot be recovered (psutil raises NoSuchProcess). The + # snapshot is stopped after the drain alongside the survivors. # - # Why launchers matter: the drain targets the PID that wrote the PID - # file (the uv-side worker). On Windows that worker's parent is usually - # the venv-side ``python.exe`` launcher, which keeps venv ``.pyd`` files - # mapped and is what ``_detect_venv_python_processes()`` reports - # downstream. Left alive, it trips the venv-holder guard and aborts the - # update even though the gateway itself is stopped. + # The drain targets the PID-file writer (uv-side worker); its parent is + # usually the venv ``python.exe`` launcher, which keeps venv ``.pyd`` + # files mapped and is what ``_detect_venv_python_processes()`` reports. + # Left alive, it trips the venv-holder guard though the gateway is stopped. launcher_pids = _m()._venv_launcher_ancestors(mapped_pids) print("→ Stopping Windows gateway process(es) before updating Hermes...") @@ -6688,11 +6107,9 @@ def _pause_windows_gateways_for_update() -> dict | None: except Exception: drain_timeout = 10.0 if socket_acks: - # A socket-paused gateway drains its ACTIVE TURN before exiting; give - # it the budget it declared (plus teardown grace) rather than only - # the local default, so a mid-turn gateway isn't force-killed at the - # end of a too-short wait — the exact outcome the verb exists to - # prevent. + # A socket-paused gateway drains its ACTIVE TURN before exiting; honor + # the budget it declared (plus teardown grace) so a mid-turn gateway + # isn't force-killed by a too-short wait. try: declared = max( float(a.get("drain_timeout") or 0.0) for a in socket_acks @@ -6714,12 +6131,10 @@ def _pause_windows_gateways_for_update() -> dict | None: if pid not in profile_processes and pid not in service_gateway_pids ] - # Snapshot each unmapped gateway's command line *before* we force-kill it, - # so ``_resume_windows_gateways_after_update`` can respawn it by replaying - # its own argv. Unmapped gateways are ones with no profile→PID-file mapping - # — e.g. a Windows Scheduled Task running ``pythonw.exe -m hermes_cli.main - # gateway run``. Without this snapshot they were force-killed and never - # restarted (the "Restart manually after update" dead-end from #50090). + # Snapshot each unmapped gateway's argv *before* force-killing it so + # ``_resume_windows_gateways_after_update`` can replay it. Unmapped = no + # profile→PID-file mapping (e.g. a Scheduled Task running ``pythonw.exe -m + # hermes_cli.main gateway run``); without this they were never restarted (#50090). unmapped: list[dict] = [] for pid in unmapped_pids: argv = None @@ -6730,10 +6145,8 @@ def _pause_windows_gateways_for_update() -> dict | None: unmapped.append({"pid": int(pid), "argv": argv}) # Stop drain survivors, unmapped gateways, and the pre-drain launcher - # snapshot. ``terminate_pid(force=True)`` is a tree kill, so a launcher - # that outlived its worker takes any stragglers with it; a launcher that - # already exited with its drained worker raises ProcessLookupError below - # and is skipped. + # snapshot. ``terminate_pid(force=True)`` is a tree kill; a launcher that + # already exited with its worker raises ProcessLookupError and is skipped. force_killed = [] for pid in sorted(set(survivors).union(unmapped_pids).union(launcher_pids)): try: @@ -6769,10 +6182,9 @@ def _pause_windows_gateways_for_update() -> dict | None: "unmapped": unmapped, } - # Stop SCM-supervised gateways only after every fallible preparation step - # for ordinary gateways is complete. From this point to return, any error - # restores both the attempted services and the already-paused ordinary - # gateways before aborting the update. + # Stop SCM-supervised gateways only after every fallible step for ordinary + # gateways is done; from here on any error restores attempted services and + # already-paused ordinary gateways before aborting. paused_services = [] current_service_name = None try: @@ -6834,24 +6246,20 @@ def _pause_windows_gateways_for_update() -> dict | None: def _cold_start_windows_gateway_after_update() -> bool: """Start a fresh detached gateway after update when one is installed but down. - Invoked from ``_resume_windows_gateways_after_update`` for the - ``cold_start_if_installed`` case: no gateway was running when the update - began, but an autostart entry (Scheduled Task / Startup-folder login item) - is installed, signalling the user wants a gateway. Unlike the relaunch - paths — which watch an old PID and respawn once it exits — this is a direct - fresh spawn via the same hidden-console + breakaway path that - ``hermes gateway start`` uses (``gateway_windows._spawn_detached``). + Called from ``_resume_windows_gateways_after_update`` for the + ``cold_start_if_installed`` case: no gateway was running at update start, + but an autostart entry is installed. Unlike the relaunch paths (watch an + old PID, respawn on exit) this is a direct spawn via the same + hidden-console + breakaway path as ``hermes gateway start`` + (``gateway_windows._spawn_detached``). Best-effort and idempotent: re-checks that nothing is running first so a - concurrent start (e.g. the autostart entry firing) can't produce a - duplicate gateway. + concurrent start (e.g. the autostart entry firing) can't duplicate. A successful ``Popen`` only proves the process was created, not that it - survived (e.g. a Windows job object denying breakaway kills it before it - logs anything — #84185). So the success line is gated on the same - post-spawn liveness poll every other ``_spawn_detached`` caller uses - (``gateway_windows._report_gateway_start``), instead of being printed - unconditionally from the returned PID. + survived (a job object denying breakaway kills it before it logs, #84185), + so the success line is gated on the same post-spawn liveness poll every + other ``_spawn_detached`` caller uses (``_report_gateway_start``). """ if not _m()._is_windows(): return True @@ -6915,6 +6323,24 @@ def _cold_start_windows_gateway_after_update() -> bool: return True +def _systemctl(cmd: list, *, timeout: float): + """Run a systemctl (or sudo systemctl) invocation, capturing utf-8 text with a timeout.""" + return subprocess.run( + cmd, + capture_output=True, + text=True, encoding="utf-8", errors="replace", + timeout=timeout, + ) + + +def _systemctl_reset_and_restart(manage_cmd: list, svc_name: str): + """``reset-failed`` then ``restart`` a unit. Always clear failed state first: if + systemd's own auto-restart attempts already parked the unit in a failed state, + a plain ``restart`` can wedge against the RestartSec backoff and leave it dead.""" + _systemctl(manage_cmd + ["reset-failed", svc_name], timeout=10) + return _systemctl(manage_cmd + ["restart", svc_name], timeout=15) + + def _for_each_systemd_gateway_unit( list_units_stdout: str, *, @@ -6936,10 +6362,9 @@ def _for_each_systemd_gateway_unit( if not unit.endswith(".service"): continue # list-units is already pattern-filtered, but keep the name gate so a - # stray non-gateway/serve line cannot enter the restart path. - # ``unit.startswith("hermes-serve")`` alone would also accept the - # unrelated ``hermes-server.service`` — require the exact base unit - # or the hyphenated profile family instead (review on #83595). + # stray line cannot enter the restart path. Require the exact base unit + # or hyphenated profile family: ``startswith("hermes-serve")`` would + # also accept the unrelated ``hermes-server.service`` (#83595). if not ( unit == "hermes-gateway.service" or unit.startswith("hermes-gateway-") @@ -6956,17 +6381,14 @@ def _for_each_systemd_gateway_unit( def _service_unit_supports_graceful_sigusr1_restart(svc_name: str) -> bool: """Whether *svc_name* wires SIGUSR1 to a graceful drain-then-restart. - Only ``hermes-gateway*`` units run ``gateway/run.py``, which installs the - SIGUSR1 handler. ``hermes-serve*`` units (#83438) don't, so sending them - SIGUSR1 would just invoke the default terminate action and burn the full - drain budget waiting for an exit that was never graceful — go straight to - the blunt ``systemctl restart`` path for those instead. + Only ``hermes-gateway*`` units run ``gateway/run.py`` (the SIGUSR1 + handler). ``hermes-serve*`` units (#83438) don't: SIGUSR1 would just + terminate them and burn the full drain budget, so they go straight to the + blunt ``systemctl restart`` path. - Uses the same strict exact/hyphenated shape as the unit-name gate in - ``_for_each_systemd_gateway_unit`` so a hypothetical near-prefix unit - (``hermes-gateway-helper`` is fine — profile units are - ``hermes-gateway-`` — but ``hermes-gatewayd``-style names are - not) can't be sent a SIGUSR1 it doesn't handle. + Same strict exact/hyphenated shape as the unit-name gate in + ``_for_each_systemd_gateway_unit``, so a near-prefix unit like + ``hermes-gatewayd`` can't be sent a SIGUSR1 it doesn't handle. """ return svc_name == "hermes-gateway" or svc_name.startswith("hermes-gateway-") @@ -6990,10 +6412,9 @@ def _warn_incomplete_gateway_fleet_restart(failed_units: list) -> None: for name in ordered: print(f" - {name}") if is_macos(): - # A launchd label reaches this list when launchd was not supervising a - # live process after the restart (#88848), so the unit is not merely - # stale — it is very likely deregistered, and `launchctl kickstart` - # cannot revive a job launchd no longer knows about. + # A launchd label lands here when launchd was not supervising a live + # process after the restart (#88848) — very likely deregistered, which + # `launchctl kickstart` cannot revive. print(" Listed services may be deregistered from launchd, or still") print(" running pre-update code (mixed sys.modules). Recover with:") print(" hermes gateway status") @@ -7016,26 +6437,19 @@ def _restart_launchd_gateway_after_update( ) -> tuple[list, list]: """Restart the invoking profile's launchd gateway after an update. - #74973 (salvage #75021 by @jeff-mettel): the restart used to be gated on - ``launchctl list