From 7a94b1fbf77123140de2c0e1d8c8eca2209ca8c2 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 13:11:35 -0500 Subject: [PATCH 1/5] fix(update): resolve the project venv as venv or .venv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `uv venv` writes `.venv` while our installers write `venv`, and every venv lookup in the update/repair paths hardcoded `venv`. On a `.venv` install `_venv_scripts_dir()` returned None, so the Windows shim-lock preflight, the quarantine, and the console-script verification all silently skipped themselves — the update walked straight into the failure they exist to catch. Adds `hermes_constants.project_venv_dir()` as the single resolver and routes both `_venv_scripts_dir()` implementations plus the two VIRTUAL_ENV call sites through it. Refs #79542 --- hermes_cli/_install_repair.py | 15 +++++++++------ hermes_cli/main.py | 12 ++++++++---- hermes_constants.py | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/hermes_cli/_install_repair.py b/hermes_cli/_install_repair.py index 9a4953bfc7906..76e78adb98583 100644 --- a/hermes_cli/_install_repair.py +++ b/hermes_cli/_install_repair.py @@ -92,7 +92,9 @@ def _resolve_install_target(root: Path) -> tuple[list[str], dict | None]: """ uv_bin = _er._find_uv_binary() if uv_bin: - env = {**os.environ, "VIRTUAL_ENV": str(root / "venv")} + from hermes_constants import project_venv_dir + + env = {**os.environ, "VIRTUAL_ENV": str(project_venv_dir(root) or root / "venv")} if _is_termux_env(env): env.pop("PYTHONPATH", None) env.pop("PYTHONHOME", None) @@ -102,13 +104,14 @@ def _resolve_install_target(root: Path) -> tuple[list[str], dict | None]: def _venv_scripts_dir(root: Path) -> Path | None: """Project venv Scripts/bin dir, when present. stdlib-only.""" - venv_dir = root / "venv" - if not venv_dir.is_dir(): - return None - # hermes_constants is stdlib-only, so the canonical layout helper is safe + # hermes_constants is stdlib-only, so the canonical layout helpers are safe # to use from this corrupted-venv repair path (#76105: never open-code # the Scripts/bin split). - from hermes_constants import venv_bin_dir + from hermes_constants import project_venv_dir, venv_bin_dir + + venv_dir = project_venv_dir(root) + if venv_dir is None: + return None scripts = venv_bin_dir(venv_dir, windows=_is_windows()) return scripts if scripts.is_dir() else None diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 7432f1f018c21..cb456a038597d 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8785,7 +8785,10 @@ def _default_venv_install_target() -> tuple[list[str], dict[str, str] | None]: except Exception: uv_bin = None if uv_bin: - env = {**os.environ, "VIRTUAL_ENV": str(PROJECT_ROOT / "venv")} + from hermes_constants import project_venv_dir + + venv_dir = project_venv_dir(PROJECT_ROOT) or PROJECT_ROOT / "venv" + env = {**os.environ, "VIRTUAL_ENV": str(venv_dir)} if _is_termux_env(env): env.pop("PYTHONPATH", None) env.pop("PYTHONHOME", None) @@ -8838,10 +8841,11 @@ def _is_windows() -> bool: def _venv_scripts_dir() -> Path | None: """Return the venv Scripts directory if we're running inside the project venv.""" - venv_dir = PROJECT_ROOT / "venv" - if not venv_dir.is_dir(): + from hermes_constants import project_venv_dir, venv_bin_dir + + venv_dir = project_venv_dir(PROJECT_ROOT) + if venv_dir is None: return None - from hermes_constants import venv_bin_dir scripts = venv_bin_dir(venv_dir, windows=_is_windows()) return scripts if scripts.is_dir() else None diff --git a/hermes_constants.py b/hermes_constants.py index 13602e20d6d10..e7af1883970e6 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -1610,6 +1610,22 @@ def venv_bin_dir(venv_dir, *, windows: bool | None = None) -> Path: return Path(venv_dir) / ("Scripts" if windows else "bin") +def project_venv_dir(project_root) -> Path | None: + """The project's venv directory, ``venv`` or ``.venv``, when one exists. + + ``uv venv`` defaults to ``.venv`` while our installers create ``venv``, so + both layouts are in the wild. Call sites that only knew about ``venv`` + silently no-oped on a ``.venv`` install — that is how the Windows + shim-lock preflight skipped itself entirely (#79542). ``venv`` wins when + both exist, matching what the installers write. + """ + for name in ("venv", ".venv"): + candidate = Path(project_root) / name + if candidate.is_dir(): + return candidate + return None + + def venv_python_path(venv_dir, *, windows: bool | None = None) -> Path: """Path to the Python interpreter inside *venv_dir* (may not exist).""" if windows is None: From 867ab54e20cf491bae62133614fa4708ccaef96d Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 13:13:09 -0500 Subject: [PATCH 2/5] fix(update): re-run Windows updates off the console shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes update` launched as venv\Scripts\hermes.exe can never finish on Windows. The launcher runs the interpreter with the shim as its script and holds it open without FILE_SHARE_DELETE for the whole command, so the quarantine rename is refused and uv fails to replace hermes.exe with os error 32 — every time, with no Desktop, gateway or AV involved. The concurrent-instance preflight cannot catch it because it excludes this process and its ancestors by design. Detect the shim from both the process ancestry and this process's own launch paths (argv[0], __main__.__file__, the spec origin — the runpy/zipapp launch puts \__main__.py there), intersected with the project venv's shims so an unrelated hermes.exe never matches. When it matches, re-run the same argv as `venv\Scripts\python.exe -m hermes_cli.main ...` and return, which releases the shim before the child installs anything. The hand-off sits ahead of the update lock so the child claims the marker itself rather than adopting one the parent immediately releases, and any failure falls through to the previous in-process behaviour with the manual command printed. Refs #88838, #89599, #86093 --- hermes_cli/main.py | 130 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 114 insertions(+), 16 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index cb456a038597d..a29bf923cbe94 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8741,38 +8741,128 @@ def _recover_core_update_marker_locked() -> None: print(f" {sys.executable} -m pip install -e '.[all]'") -def _windows_running_hermes_launcher_locked() -> bool: - """True when a venv ``hermes*.exe`` shim is this process or an ancestor. +def _norm_exe_path(path) -> str: + """Case-folded resolved path, for comparing executables on Windows.""" + try: + return str(Path(path).resolve()).lower() + except OSError: + return str(path).lower() - Best-effort: returns False when psutil is unavailable or inspection fails. + +def _windows_shim_in_process_chain() -> Path | None: + """The venv console shim this process runs from or under, if any. + + ``venv\\Scripts\\hermes.exe`` is a launcher that runs the interpreter with + the shim itself as its script, and that keeps the shim open — without + ``FILE_SHARE_DELETE`` — for the whole process lifetime. So every + ``hermes ...`` command holds its own shim, and an editable install run + from one can never rewrite it (#88838, #89599). + + Two independent probes, because either can come up empty. Process + ancestry finds the launcher when it is a separate parent process, but + needs psutil. This process's own launch paths (``sys.argv[0]``, + ``__main__.__file__``, the module spec origin) cover the rest — the + runpy/zipapp launch puts ``\\__main__.py`` there, which a plain + argv[0] check misses. + + Candidates are intersected with the project venv's own shims, so a + ``hermes.exe`` belonging to some other install never matches. """ if not _is_windows(): - return False + return None scripts_dir = _venv_scripts_dir() if scripts_dir is None: - return False - shims = _hermes_exe_shims(scripts_dir) + return None + shims = {_norm_exe_path(shim): shim for shim in _hermes_exe_shims(scripts_dir)} if not shims: - return False - shim_set: set[str] = set() - for shim in shims: - try: - shim_set.add(str(shim.resolve()).lower()) - except OSError: - shim_set.add(str(shim).lower()) + return None + + def _match(candidate) -> Path | None: + path = Path(candidate) + if path.name.lower() == "__main__.py": + path = path.parent + return shims.get(_norm_exe_path(path)) + + candidates: list[str] = list(sys.argv[:1]) + main_mod = sys.modules.get("__main__") + for attr in (getattr(main_mod, "__file__", None), + getattr(getattr(main_mod, "__spec__", None), "origin", None)): + if attr: + candidates.append(attr) + for candidate in candidates: + matched = _match(candidate) + if matched is not None: + return matched + try: import psutil me = psutil.Process() for proc in [me] + list(me.parents()): try: - exe_norm = str(Path(proc.exe()).resolve()).lower() + matched = _match(proc.exe()) except Exception: continue - if exe_norm in shim_set: - return True + if matched is not None: + return matched except Exception: + return None + return None + + +def _windows_running_hermes_launcher_locked() -> bool: + """True when a venv ``hermes*.exe`` shim is this process or an ancestor. + + Best-effort: returns False when psutil is unavailable or inspection fails. + """ + return _windows_shim_in_process_chain() is not None + + +# Set on the re-exec'd child so it can never spawn another one. +_UPDATE_REEXEC_ENV = "HERMES_UPDATE_REEXEC" + + +def _reexec_update_off_windows_shim() -> bool: + """Hand this update to the venv interpreter, off the console shim. + + Returns True when a child was spawned and the caller must return at once, + so this process exits and releases the shim before the child reaches + ``pip install -e .``. Returns False to continue in-process. + + The child is spawned, not waited on — this process exiting IS the fix, so + the shell sees the spawn's status rather than the update's. The update + prints its own result, and ``--gateway`` writes the true exit code to + ``.update_exit_code`` for the gateway watcher before restarting. + + Anything that stops the hand-off (no venv python, spawn refused) falls + through to the old in-process behaviour with the manual command printed, + so a broken venv still gets whatever the update can do rather than a + dead end. + """ + if os.environ.get(_UPDATE_REEXEC_ENV) == "1": + return False + shim = _windows_shim_in_process_chain() + if shim is None: return False + + from hermes_constants import venv_python_path + + python_exe = venv_python_path(shim.parent.parent, windows=True) + cmd = [str(python_exe), "-m", "hermes_cli.main", *sys.argv[1:]] + if python_exe.is_file(): + try: + subprocess.Popen(cmd, env={**os.environ, _UPDATE_REEXEC_ENV: "1"}) + print( + f"→ Windows: {shim.name} cannot replace itself while it runs; " + "continuing the update under the venv Python." + ) + print(" Progress continues below; this shell returns immediately.") + return True + except OSError as exc: + logger.debug("Update re-exec via %s failed: %s", python_exe, exc) + print(f" ⚠ Could not re-run the update off {shim.name}. If the install") + print(" fails to replace it, run this from a fresh shell instead:") + print(f" {subprocess.list2cmdline(cmd)}") return False @@ -9968,6 +10058,14 @@ def cmd_update(args): ) return + # Windows: an update launched through venv\Scripts\hermes.exe holds that + # shim open for its whole run, and the dependency sync has to replace it. + # Hand off to the venv interpreter before anything else — in particular + # before the update lock, so the child claims the marker itself instead of + # adopting one this process is about to release. + if _reexec_update_off_windows_shim(): + return + gateway_mode = getattr(args, "gateway", False) # Protect against mid-update terminal disconnects (SIGHUP) and tolerate From a9eb99d1724a9efa4ad6723bbd46f930a2478889 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 13:14:33 -0500 Subject: [PATCH 3/5] fix(update): stop deferring shim renames to next boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MOVEFILE_DELAY_UNTIL_REBOOT was the quarantine's last resort, and it is worse than doing nothing. It writes to HKLM, so a non-elevated update — every Desktop-driven one, and most terminal ones — gets ERROR_ACCESS_DENIED and reports nothing. When it does succeed it frees nothing for the install running right now, and the queued operation outlives that update: at the next boot it moves aside whatever sits at the shim path, including a shim a later repair just wrote. Drops the fallback and sweeps entries older versions queued, matching only our own -> .old. pairs so unrelated installers keep theirs. Salvaged from #88121 by @fangliquanflq. --- hermes_cli/main.py | 142 +++++++++++++++++------------ hermes_cli/subcommands/update.py | 2 +- scripts/desktop-update/windows.ps1 | 14 +-- 3 files changed, 90 insertions(+), 68 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index a29bf923cbe94..8402f228de94a 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8980,19 +8980,15 @@ def _quarantine_running_hermes_exe( 1. Retry up to ``max_attempts`` times with exponential backoff (100/250/500/1000 ms). Handles the AV-scanner case. - 2. If all retries fail, schedule the .exe for replacement on next - reboot via ``MoveFileExW(MOVEFILE_DELAY_UNTIL_REBOOT)``. This still - lets uv create a fresh shim at the original path (Windows will keep - the old file's content under a new name until the reboot), so the - update can complete; the user just needs to reboot to fully unload - the stale image. - 3. Print a clear warning naming the most likely culprit (running - Hermes Desktop / gateway / REPL) and pointing to ``--force``. + 2. If all retries fail, print a clear warning naming the most likely + culprit (running Hermes Desktop / gateway / REPL). + + The updater's own launcher is no longer one of those culprits: an update + started from ``hermes.exe`` re-runs itself under the venv Python before + reaching here (``_reexec_update_off_windows_shim``). Returns the list of (original, quarantined) pairs so the caller can roll - back if the install itself fails before uv writes a replacement. Pairs - where we used ``MOVEFILE_DELAY_UNTIL_REBOOT`` are NOT returned — they - are already deferred and roll-back is meaningless. + back if the install itself fails before uv writes a replacement. """ moved: list[tuple[Path, Path]] = [] if not _is_windows(): @@ -9028,27 +9024,13 @@ def _quarantine_running_hermes_exe( if last_exc is None: continue - # All in-process renames failed. Try MoveFileEx with - # MOVEFILE_DELAY_UNTIL_REBOOT as a last resort. This succeeds in the - # exact case where the inline rename failed (another process holds - # the handle without share-delete), at the cost of requiring a - # reboot to fully reclaim the old .exe. - scheduled = _schedule_replace_on_reboot(shim, target) - if scheduled: - print( - f" ⚠ {shim.name} is locked by another process; scheduled " - f"replacement on next reboot." - ) - print( - " The new shim was written at the same path, but a " - "reboot is needed to fully unload the old one." - ) - # Do NOT append to ``moved``: we don't want roll-back to undo a - # reboot-deferred operation. - continue - - # Truly couldn't budge the .exe. Print an actionable warning and let - # uv try its luck — sometimes uv's own retry handling pulls through. + # Every rename failed. Deferring one to next boot via + # MOVEFILE_DELAY_UNTIL_REBOOT used to be the fallback here, but it + # cannot help: it needs elevation we don't have, and when it does + # land it frees nothing for the install running right now while + # queueing an operation that will move a later, freshly repaired shim + # aside at next boot. Report and let uv try its luck instead — + # sometimes its own retry handling pulls through. print( f" ⚠ Could not quarantine {shim.name} ({last_exc.__class__.__name__}: " f"another process is holding it open)." @@ -9061,39 +9043,78 @@ def _quarantine_running_hermes_exe( return moved -def _schedule_replace_on_reboot(shim: Path, quarantine_target: Path) -> bool: - """Schedule ``shim`` -> ``quarantine_target`` via PendingFileRenameOperations. +_PENDING_RENAME_KEY = r"SYSTEM\CurrentControlSet\Control\Session Manager" +_PENDING_RENAME_VALUE = "PendingFileRenameOperations" - Uses Win32 ``MoveFileExW`` with ``MOVEFILE_REPLACE_EXISTING | - MOVEFILE_DELAY_UNTIL_REBOOT``. The OS persists the rename in - ``HKLM\\System\\CurrentControlSet\\Control\\Session Manager\\ - PendingFileRenameOperations`` and applies it before any user-mode code - runs on next boot — at which point no process can hold the .exe. - Returns ``True`` if the schedule call succeeded, ``False`` otherwise - (non-Windows, ctypes failure, lack of privilege, etc.). Never raises. +def _filter_pending_shim_renames( + entries: list[str], shims: list[Path] +) -> tuple[list[str], int]: + """Drop shim-quarantine pairs from a PendingFileRenameOperations value. + + The value is a flat REG_MULTI_SZ of (source, target) pairs, and other + installers share it, so only pairs matching our own + ```` -> ``.old.`` naming are removed. Returns the + entries to keep and how many pairs were dropped. """ - if not _is_windows(): - return False - try: - import ctypes - from ctypes import wintypes + import ntpath - MOVEFILE_REPLACE_EXISTING = 0x1 - MOVEFILE_DELAY_UNTIL_REBOOT = 0x4 + def _norm(value: str) -> str: + path = str(value).lstrip("!") + if path.startswith("\\??\\"): + path = path[4:] + return ntpath.normcase(ntpath.normpath(path)) - MoveFileExW = ctypes.windll.kernel32.MoveFileExW - MoveFileExW.argtypes = [wintypes.LPCWSTR, wintypes.LPCWSTR, wintypes.DWORD] - MoveFileExW.restype = wintypes.BOOL + shim_paths = {_norm(str(shim)) for shim in shims} + kept: list[str] = [] + removed = 0 + for index in range(0, len(entries) - 1, 2): + source, target = entries[index], entries[index + 1] + source_norm = _norm(source) + if source_norm in shim_paths and _norm(target).startswith(f"{source_norm}.old."): + removed += 1 + else: + kept.extend((source, target)) + if len(entries) % 2: + kept.append(entries[-1]) + return kept, removed - ok = MoveFileExW( - str(shim), - str(quarantine_target), - MOVEFILE_REPLACE_EXISTING | MOVEFILE_DELAY_UNTIL_REBOOT, - ) - return bool(ok) - except Exception: - return False + +def _cleanup_pending_shim_renames(scripts_dir: Path) -> int: + """Drop reboot renames older Hermes versions queued for our shims. + + Hermes used to fall back to ``MoveFileExW(MOVEFILE_DELAY_UNTIL_REBOOT)`` + when the quarantine rename failed. Those entries outlive the update that + queued them, so at the next boot they move away whatever now sits at the + shim path — including a shim a later repair just wrote. Needs elevation + to remove (same as it needed to create); a no-op otherwise. + """ + if not _is_windows(): + return 0 + try: + import winreg + + with winreg.OpenKey( + winreg.HKEY_LOCAL_MACHINE, + _PENDING_RENAME_KEY, + 0, + winreg.KEY_QUERY_VALUE | winreg.KEY_SET_VALUE, + ) as key: + entries, value_type = winreg.QueryValueEx(key, _PENDING_RENAME_VALUE) + if value_type != winreg.REG_MULTI_SZ or not isinstance(entries, list): + return 0 + kept, removed = _filter_pending_shim_renames( + entries, _hermes_exe_shims(scripts_dir) + ) + if not removed: + return 0 + if kept: + winreg.SetValueEx(key, _PENDING_RENAME_VALUE, 0, winreg.REG_MULTI_SZ, kept) + else: + winreg.DeleteValue(key, _PENDING_RENAME_VALUE) + return removed + except (OSError, ValueError): + return 0 def _restore_quarantined_exes(moved: list[tuple[Path, Path]]) -> None: @@ -9146,7 +9167,7 @@ def _run_quarantined_install( def _cleanup_quarantined_exes(scripts_dir: Path | None = None) -> None: - """Sweep ``hermes.exe.old.*`` left by prior updates. + """Sweep ``hermes.exe.old.*`` and stale reboot renames left by prior updates. Called early on every hermes invocation. The .old files are unlocked once their owning process exited, so deletion succeeds the next run. Silent @@ -9158,6 +9179,7 @@ def _cleanup_quarantined_exes(scripts_dir: Path | None = None) -> None: scripts_dir = _venv_scripts_dir() if scripts_dir is None: return + _cleanup_pending_shim_renames(scripts_dir) try: for stale in scripts_dir.glob("*.exe.old.*"): try: diff --git a/hermes_cli/subcommands/update.py b/hermes_cli/subcommands/update.py index f612001fa0ee0..680693c393c07 100644 --- a/hermes_cli/subcommands/update.py +++ b/hermes_cli/subcommands/update.py @@ -65,7 +65,7 @@ def build_update_parser(subparsers, *, cmd_update: Callable) -> None: "--force", action="store_true", default=False, - help="Windows: proceed with the update even when another hermes.exe is detected. The concurrent process will likely cause WinError 32 warnings and may leave a reboot-deferred .exe replacement. Does NOT bypass the venv-process guard (see --force-venv).", + help="Windows: proceed with the update even when another hermes.exe is detected. The concurrent process will likely cause WinError 32 warnings. Does NOT bypass the venv-process guard (see --force-venv).", ) update_parser.add_argument( "--force-venv", diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 247ca6cf5bc70..c5e68f85cd9de 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -703,13 +703,13 @@ try { # the next. Step 2's preflight cannot catch it, because the shim genuinely # IS unlocked at that moment. # - # When the rename loses that race, _schedule_replace_on_reboot is the last - # resort -- and it writes to HKLM\...\PendingFileRenameOperations, which - # requires elevation. A Desktop-driven update runs non-elevated, so it - # returns ERROR_ACCESS_DENIED and `uv pip install -e .` exits 2. The ZIP - # fallback repeats the identical sequence, so the desktop build stage is - # never reached and apps/desktop/release is left missing -- an install whose - # Start Menu shortcut points at a Hermes.exe that no longer exists. + # When the rename loses that race there is no recovery: `uv pip install -e .` + # exits 2 and the ZIP fallback repeats the identical sequence, so the desktop + # build stage is never reached and apps/desktop/release is left missing -- an + # install whose Start Menu shortcut points at a Hermes.exe that no longer + # exists. (A reboot-deferred rename was the old last resort here; it needed + # elevation a Desktop-driven update does not have, and freed nothing for the + # install already in flight.) # # Running the same code as `python.exe -m hermes_cli.main update` puts the # inherited handles on python.exe, which uv never has to replace. From 3f6d4c6338b258245da98c1ab9f1537352bb6a32 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 13:15:02 -0500 Subject: [PATCH 4/5] fix(gateway): run Windows /update as a module, not through the shim The Windows branch spawned the updater as `hermes.exe update --gateway`, so the update held the very shim it had to replace and failed with os error 32. Invoke it as `python -m hermes_cli.main update --gateway` under the same interpreter the gateway already runs, which maps no shim. Salvaged from #89970 by @Akloenx123. --- gateway/slash_commands.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 1986bbd374a27..cd54164bd0d9b 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -5958,8 +5958,11 @@ async def _handle_update_command(self, event: MessageEvent) -> str: import textwrap from hermes_cli._subprocess_compat import windows_detach_popen_kwargs - # hermes_cmd is a list of argv parts we can pass directly - # (no shell-quoting needed). + # Invoke the updater as a module under this interpreter rather + # than through hermes_cmd (venv\Scripts\hermes.exe): the shim + # launcher holds its own file open for the whole run, and the + # update has to replace it. Going through python.exe maps no + # shim, so the entry points can be rewritten freely. helper = textwrap.dedent( """ import os, subprocess, sys @@ -5979,7 +5982,8 @@ async def _handle_update_command(self, event: MessageEvent) -> str: [ sys.executable, "-c", helper, str(output_path), str(exit_code_path), - *hermes_cmd, "update", "--gateway", + sys.executable, "-m", "hermes_cli.main", + "update", "--gateway", ], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, From 268615cbb3aad3a36aec37151f5c6ef1652494e4 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 13:17:20 -0500 Subject: [PATCH 5/5] test(update): cover the Windows shim self-lock class Detection across every launch variant (argv[0], the zipapp __main__.py, the main-module spec origin, the ancestor chain) plus the venv scoping that keeps an unrelated hermes.exe from triggering a hand-off; the re-exec's argv, env marker, loop guard and both fall-through paths; the pending-rename filter; and the venv/.venv layout split. Retires the reboot-deferred quarantine assertion along with the fallback. Launch-variant cases from #89970 by @Akloenx123, pending-rename cases from #88121 by @fangliquanflq. --- .../test_update_concurrent_quarantine.py | 34 +-- .../hermes_cli/test_update_shim_self_lock.py | 224 ++++++++++++++++++ 2 files changed, 237 insertions(+), 21 deletions(-) create mode 100644 tests/hermes_cli/test_update_shim_self_lock.py diff --git a/tests/hermes_cli/test_update_concurrent_quarantine.py b/tests/hermes_cli/test_update_concurrent_quarantine.py index 784c9f9bc4110..7c6aef5f5f4b7 100644 --- a/tests/hermes_cli/test_update_concurrent_quarantine.py +++ b/tests/hermes_cli/test_update_concurrent_quarantine.py @@ -139,7 +139,7 @@ def test_detect_concurrent_parents_call_robust_to_one_bad_hop(_winp, tmp_path): # --------------------------------------------------------------------------- -# _quarantine_running_hermes_exe — retry + reboot-deferred fallback +# _quarantine_running_hermes_exe — retry, then report # --------------------------------------------------------------------------- @@ -160,36 +160,26 @@ def test_quarantine_succeeds_first_attempt(_winp, tmp_path): @patch.object(cli_main, "_is_windows", return_value=True) -def test_quarantine_falls_back_to_reboot_schedule(_winp, tmp_path, capsys, monkeypatch): - """When every retry fails, we schedule via MoveFileEx and warn helpfully.""" +def test_quarantine_reports_a_lock_it_cannot_break(_winp, tmp_path, capsys, monkeypatch): + """Every retry failed: name the likely culprits, queue nothing for reboot.""" shim = tmp_path / "hermes.exe" shim.write_bytes(b"locked") def always_fails(self, target): raise OSError(32, "The process cannot access the file (simulated lock)") - scheduled_calls: list[tuple[Path, Path]] = [] - - def fake_schedule(s: Path, q: Path) -> bool: - scheduled_calls.append((s, q)) - return True - monkeypatch.setattr(cli_main, "_hermes_exe_shims", lambda d: [shim]) - with patch.object(Path, "rename", always_fails), patch.object( - cli_main, "_schedule_replace_on_reboot", fake_schedule - ), patch("time.sleep", lambda *_a, **_k: None): + with patch.object(Path, "rename", always_fails), patch( + "time.sleep", lambda *_a, **_k: None + ): pairs = cli_main._quarantine_running_hermes_exe(tmp_path) - captured = capsys.readouterr().out + captured = capsys.readouterr().out.lower() - # The reboot-deferred path was used. - assert scheduled_calls and scheduled_calls[0][0] == shim - # It is NOT added to the returned roll-back list (the issue calls this - # out — don't undo a deferred operation). assert pairs == [] - # The user got a clear message, not raw [WinError 32]. - assert "scheduled" in captured.lower() - assert "reboot" in captured.lower() + # A clear message, not raw [WinError 32], and no reboot promise we can't keep. + assert "could not quarantine" in captured + assert "reboot" not in captured @@ -258,7 +248,9 @@ def fake_wait(pids, *, timeout): assert waited_for == [101] assert terminated == [(202, True)] - marker = json.loads((profile_home / ".gateway-planned-stop.json").read_text()) + marker = json.loads( + (profile_home / ".gateway-planned-stop.json").read_text(encoding="utf-8") + ) assert marker["target_pid"] == 101 assert marker["stopper_pid"] == os.getpid() diff --git a/tests/hermes_cli/test_update_shim_self_lock.py b/tests/hermes_cli/test_update_shim_self_lock.py new file mode 100644 index 0000000000000..5bcc417bdce64 --- /dev/null +++ b/tests/hermes_cli/test_update_shim_self_lock.py @@ -0,0 +1,224 @@ +"""The Windows console-shim update self-lock (#88838, #89599, #86093). + +``venv\\Scripts\\hermes.exe`` is a launcher that runs the interpreter with the +shim itself as its script, keeping the file open without FILE_SHARE_DELETE for +the whole command. An update started that way must therefore replace a file it +is holding, which Windows refuses — so ``hermes update`` re-runs itself under +``venv\\Scripts\\python.exe`` before touching anything. + +``_is_windows`` is patched so these paths are exercised on any host. +""" + +from __future__ import annotations + +import sys +import types +from pathlib import Path + +import pytest + +from hermes_cli import main as cli_main + +SHIM_NAMES = ["hermes.exe", "hermes-agent.exe", "hermes-acp.exe", "hermes-gateway.exe"] + + +@pytest.fixture +def venv(tmp_path, monkeypatch): + """A Windows-shaped project venv with a python.exe, wired into main.""" + scripts = tmp_path / "venv" / "Scripts" + scripts.mkdir(parents=True) + (scripts / "python.exe").write_bytes(b"") + monkeypatch.setattr(cli_main, "_is_windows", lambda: True) + monkeypatch.setattr(cli_main, "_venv_scripts_dir", lambda: scripts) + monkeypatch.setattr(sys, "argv", ["hermes", "update"]) + monkeypatch.delenv(cli_main._UPDATE_REEXEC_ENV, raising=False) + _fake_psutil(monkeypatch, []) + return scripts + + +def _fake_psutil(monkeypatch, ancestor_exes: list[str]): + """Stand in for psutil with a fixed self+ancestor executable chain.""" + + class _Proc: + def __init__(self, exe=None): + self._exe = exe + + def exe(self): + if self._exe is None: + raise OSError("exe unavailable") + return self._exe + + def parents(self): + return [_Proc(exe) for exe in ancestor_exes] + + monkeypatch.setitem(sys.modules, "psutil", types.SimpleNamespace(Process=_Proc)) + + +def _capture_popen(monkeypatch, raises: Exception | None = None): + calls = [] + + def fake_popen(cmd, env=None, **kwargs): + if raises is not None: + raise raises + calls.append((list(cmd), dict(env or {}))) + return object() + + monkeypatch.setattr(cli_main.subprocess, "Popen", fake_popen) + return calls + + +# --------------------------------------------------------------------------- +# Shim detection +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("shim_name", SHIM_NAMES) +def test_detects_shim_as_argv0(venv, monkeypatch, shim_name): + monkeypatch.setattr(sys, "argv", [str(venv / shim_name), "update"]) + assert cli_main._windows_shim_in_process_chain() == venv / shim_name + + +def test_detects_shim_from_zipapp_main_py(venv, monkeypatch): + """runpy/zipapp launches put ``\\__main__.py`` in argv[0].""" + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe" / "__main__.py")]) + assert cli_main._windows_shim_in_process_chain() == venv / "hermes.exe" + + +def test_detects_shim_from_main_module_spec_origin(venv, monkeypatch): + fake_main = types.SimpleNamespace( + __file__=None, + __spec__=types.SimpleNamespace(origin=str(venv / "hermes.exe")), + ) + monkeypatch.setitem(sys.modules, "__main__", fake_main) + assert cli_main._windows_shim_in_process_chain() == venv / "hermes.exe" + + +def test_detects_shim_in_ancestor_chain(venv, monkeypatch): + """The launcher is usually a separate parent process, not argv[0].""" + _fake_psutil(monkeypatch, [str(venv / "hermes.exe")]) + assert cli_main._windows_shim_in_process_chain() == venv / "hermes.exe" + + +def test_ignores_hermes_exe_outside_the_project_venv(venv, monkeypatch, tmp_path): + """A shim from some other install must never trigger a re-exec.""" + other = tmp_path / "other" / "Scripts" + other.mkdir(parents=True) + monkeypatch.setattr(sys, "argv", [str(other / "hermes.exe"), "update"]) + _fake_psutil(monkeypatch, [str(other / "hermes.exe")]) + assert cli_main._windows_shim_in_process_chain() is None + + +def test_no_shim_off_windows(venv, monkeypatch): + monkeypatch.setattr(cli_main, "_is_windows", lambda: False) + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"]) + assert cli_main._windows_shim_in_process_chain() is None + + +def test_no_shim_without_a_venv(venv, monkeypatch): + monkeypatch.setattr(cli_main, "_venv_scripts_dir", lambda: None) + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"]) + assert cli_main._windows_shim_in_process_chain() is None + + +# --------------------------------------------------------------------------- +# Re-exec hand-off +# --------------------------------------------------------------------------- + + +def test_reexec_runs_same_args_under_venv_python(venv, monkeypatch, capsys): + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update", "--yes"]) + calls = _capture_popen(monkeypatch) + + assert cli_main._reexec_update_off_windows_shim() is True + cmd, env = calls[0] + assert cmd == [ + str(venv / "python.exe"), "-m", "hermes_cli.main", "update", "--yes", + ] + assert env[cli_main._UPDATE_REEXEC_ENV] == "1" + assert "under the venv Python" in capsys.readouterr().out + + +def test_reexec_does_not_recurse(venv, monkeypatch): + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"]) + monkeypatch.setenv(cli_main._UPDATE_REEXEC_ENV, "1") + calls = _capture_popen(monkeypatch) + + assert cli_main._reexec_update_off_windows_shim() is False + assert calls == [] + + +def test_reexec_skipped_when_not_launched_from_a_shim(venv, monkeypatch): + calls = _capture_popen(monkeypatch) + assert cli_main._reexec_update_off_windows_shim() is False + assert calls == [] + + +def test_reexec_falls_through_when_venv_python_is_missing(venv, monkeypatch, capsys): + (venv / "python.exe").unlink() + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"]) + + assert cli_main._reexec_update_off_windows_shim() is False + assert "-m hermes_cli.main update" in capsys.readouterr().out + + +def test_reexec_falls_through_when_spawn_fails(venv, monkeypatch, capsys): + monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"]) + _capture_popen(monkeypatch, raises=OSError("no exec")) + + assert cli_main._reexec_update_off_windows_shim() is False + assert "-m hermes_cli.main update" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# Reboot-deferred renames +# --------------------------------------------------------------------------- + + +def test_reboot_deferred_rename_fallback_is_gone(): + """MOVEFILE_DELAY_UNTIL_REBOOT needed elevation and freed nothing.""" + assert not hasattr(cli_main, "_schedule_replace_on_reboot") + + +def test_pending_rename_filter_drops_only_our_shim_pairs(): + shims = [Path(r"C:\hermes\venv\Scripts\hermes.exe")] + entries = [ + r"\??\C:\other\thing.dll", r"!\??\C:\other\thing.dll.bak", + r"\??\C:\hermes\venv\Scripts\hermes.exe", + r"!\??\C:\hermes\venv\Scripts\hermes.exe.old.1755624735000", + ] + kept, removed = cli_main._filter_pending_shim_renames(entries, shims) + assert removed == 1 + assert kept == entries[:2] + + +def test_pending_rename_filter_keeps_a_shim_pair_with_a_foreign_target(): + shims = [Path(r"C:\hermes\venv\Scripts\hermes.exe")] + entries = [ + r"\??\C:\hermes\venv\Scripts\hermes.exe", r"!\??\C:\somewhere\else.exe", + ] + kept, removed = cli_main._filter_pending_shim_renames(entries, shims) + assert removed == 0 + assert kept == entries + + +def test_pending_rename_filter_preserves_a_trailing_delete_entry(): + """A bare source with an empty target is a scheduled delete, not a pair.""" + entries = [r"\??\C:\other\thing.dll", "", r"\??\C:\other\orphan.dll"] + kept, removed = cli_main._filter_pending_shim_renames(entries, []) + assert removed == 0 + assert kept == entries + + +# --------------------------------------------------------------------------- +# venv layout +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("venv_name", ["venv", ".venv"]) +def test_venv_scripts_dir_finds_both_layouts(tmp_path, monkeypatch, venv_name): + """uv writes .venv; our installers write venv. Both must resolve (#79542).""" + scripts = tmp_path / venv_name / "Scripts" + scripts.mkdir(parents=True) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", tmp_path) + monkeypatch.setattr(cli_main, "_is_windows", lambda: True) + assert cli_main._venv_scripts_dir() == scripts