fix(cli-update): exclude launcher-shim ancestor PIDs from concurrent check on Windows (#29341)

hermes_cli/main.py

@@ -7253,6 +7253,60 @@ def _hermes_exe_shims(scripts_dir: Path) -> list[Path]:
     ]
 
 
+def _walk_shim_ancestor_pids(psutil_mod, shim_paths: set[str]) -> set[int]:
+    """Collect PIDs of ancestor processes whose ``exe`` matches a shim path.
+
+    On Windows the ``hermes`` console-script is a distlib-generated
+    ``Scripts\\hermes.exe`` launcher that spawns a child ``python.exe`` and
+    waits for it. Detection runs inside that child, where ``os.getpid()``
+    returns the *child* PID — so the launcher's PID is left dangling in
+    ``psutil.process_iter`` and gets reported as "another hermes.exe" even
+    though it's the very shim that started this invocation (issue #29341).
+
+    Walk the ancestor chain and treat every consecutive ancestor whose
+    executable resolves to one of our shim paths as part of the same
+    logical invocation. Stop at the first non-shim ancestor so a legitimate
+    second ``hermes`` somewhere in the tree (e.g. a Hermes Desktop child
+    sitting *under* a non-Hermes parent) is still flagged.
+
+    The walk is bounded (max 16 hops) so a misbehaving ``parent()`` chain
+    can't loop us forever. Every psutil/OS error is swallowed: this is a
+    best-effort assist for the existing ``os.getpid()`` exclusion.
+    """
+    extra: set[int] = set()
+    if not shim_paths:
+        return extra
+    try:
+        proc = psutil_mod.Process()
+    except Exception:
+        return extra
+    for _ in range(16):
+        try:
+            parent = proc.parent()
+        except Exception:
+            break
+        if parent is None:
+            break
+        try:
+            parent_exe = parent.exe()
+        except Exception:
+            break
+        if not parent_exe:
+            break
+        try:
+            parent_exe_norm = str(Path(parent_exe).resolve()).lower()
+        except (OSError, ValueError):
+            parent_exe_norm = str(parent_exe).lower()
+        if parent_exe_norm not in shim_paths:
+            break
+        try:
+            extra.add(int(parent.pid))
+        except Exception:
+            break
+        proc = parent
+    return extra
+
+
 def _detect_concurrent_hermes_instances(
     scripts_dir: Path, *, exclude_pid: int | None = None
 ) -> list[tuple[int, str]]:
@@ -7267,8 +7321,9 @@ def _detect_concurrent_hermes_instances(
 
     This helper enumerates processes whose ``exe`` matches one of the venv's
     shims (``hermes.exe`` / ``hermes-gateway.exe``) and returns ``(pid,
-    process_name)`` pairs. The caller's own PID is excluded so the running
-    ``hermes update`` invocation never reports itself.
+    process_name)`` pairs. The caller's own PID is excluded, and so is the
+    chain of immediate ancestor shims that spawned the current Python
+    process (the distlib console-script launcher — see issue #29341).
 
     Returns an empty list off-Windows, on missing psutil, or when no other
     instances exist. Never raises — process enumeration is best-effort.
@@ -7281,8 +7336,8 @@ def _detect_concurrent_hermes_instances(
     except Exception:
         return []
 
-    if exclude_pid is None:
-        exclude_pid = os.getpid()
+    seed_pid = exclude_pid if exclude_pid is not None else os.getpid()
+    exclude_pids: set[int] = {seed_pid}
 
     # Resolve every shim path to its canonical form once for cheap comparison.
     shim_paths: set[str] = set()
@@ -7294,6 +7349,12 @@ def _detect_concurrent_hermes_instances(
     if not shim_paths:
         return []
 
+    # Exclude the launcher-shim ancestor chain so the very ``hermes.exe``
+    # that started this invocation isn't reported as a concurrent peer
+    # (#29341). Without this, ``hermes update`` would always require
+    # ``--force`` on Windows even when no other Hermes process is alive.
+    exclude_pids.update(_walk_shim_ancestor_pids(psutil, shim_paths))
+
     matches: list[tuple[int, str]] = []
     try:
         proc_iter = psutil.process_iter(["pid", "exe", "name"])
@@ -7307,7 +7368,7 @@ def _detect_concurrent_hermes_instances(
             continue
         pid = info.get("pid")
         exe = info.get("exe")
-        if not exe or pid is None or pid == exclude_pid:
+        if not exe or pid is None or pid in exclude_pids:
             continue
         try:
             exe_norm = str(Path(exe).resolve()).lower()

tests/hermes_cli/test_update_concurrent_quarantine.py

@@ -118,6 +118,247 @@ def test_detect_concurrent_is_noop_off_windows(_winp, tmp_path):
     assert cli_main._detect_concurrent_hermes_instances(tmp_path) == []
 
 
+# ---------------------------------------------------------------------------
+# Launcher-shim ancestor exclusion (issue #29341)
+# ---------------------------------------------------------------------------
+#
+# On Windows ``hermes`` is a distlib-generated ``Scripts\\hermes.exe`` console
+# launcher that spawns a child ``python.exe`` and stays alive waiting on it.
+# Detection runs inside the *child* (where ``os.getpid()`` lives), so the
+# launcher PID is left in ``process_iter`` and gets reported as "another
+# hermes.exe is running". The fix walks the parent chain and excludes every
+# consecutive ancestor whose ``exe`` resolves to one of the shim paths.
+
+
+def _fake_psutil_with_chain(
+    procs, *, parent_chain, my_pid: int | None = None,
+):
+    """Build a psutil stand-in with both ``process_iter`` and ancestor walk.
+
+    ``parent_chain`` is the list of ``(pid, exe)`` returned by
+    ``Process().parent()...parent()`` starting from the current process.
+    The first element is the immediate parent; an empty list means the
+    current process has no parent (terminates the walk).
+    """
+    if my_pid is None:
+        my_pid = os.getpid()
+
+    class _FakeProc:
+        def __init__(self, pid, exe):
+            self.pid = pid
+            self._exe = exe
+            self._idx = -1  # index into parent_chain
+
+        def exe(self):
+            return self._exe
+
+        def parent(self):
+            nxt = self._idx + 1
+            if nxt >= len(parent_chain):
+                return None
+            pid, exe = parent_chain[nxt]
+            p = _FakeProc(pid, exe)
+            p._idx = nxt
+            return p
+
+    def _Process(pid=None):
+        return _FakeProc(my_pid, None)
+
+    return types.SimpleNamespace(
+        process_iter=lambda attrs: iter(procs),
+        Process=_Process,
+        Error=Exception,
+        NoSuchProcess=Exception,
+        AccessDenied=Exception,
+    )
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_excludes_launcher_shim_parent(_winp, tmp_path):
+    """The distlib ``hermes.exe`` launcher (parent of this Python) is excluded.
+
+    Reproduces the exact scenario from #29341: PowerShell → hermes.exe (PID
+    18608) → python.exe (current). Without the parent-chain walk, 18608
+    would be flagged as a concurrent peer every time.
+    """
+    scripts_dir = tmp_path
+    shim = scripts_dir / "hermes.exe"
+    shim.write_bytes(b"")
+    launcher_pid = os.getpid() + 1
+    procs = [
+        _make_proc(launcher_pid, str(shim), "hermes.exe"),
+    ]
+    fake = _fake_psutil_with_chain(
+        procs,
+        parent_chain=[(launcher_pid, str(shim))],
+    )
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    assert result == []
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_still_flags_unrelated_hermes_peer(_winp, tmp_path):
+    """Excluding the launcher ancestor must NOT mask genuinely concurrent peers.
+
+    A second ``hermes.exe`` somewhere else in the process table (e.g. Hermes
+    Desktop's backend child, or a second terminal) is a sibling, not an
+    ancestor — the walk must not exclude it.
+    """
+    scripts_dir = tmp_path
+    shim = scripts_dir / "hermes.exe"
+    shim.write_bytes(b"")
+    launcher_pid = os.getpid() + 1
+    real_peer_pid = os.getpid() + 2  # NOT in the parent chain
+    procs = [
+        _make_proc(launcher_pid, str(shim), "hermes.exe"),
+        _make_proc(real_peer_pid, str(shim), "hermes.exe"),
+    ]
+    fake = _fake_psutil_with_chain(
+        procs,
+        parent_chain=[(launcher_pid, str(shim))],
+    )
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    assert result == [(real_peer_pid, "hermes.exe")]
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_walks_multiple_shim_ancestors(_winp, tmp_path):
+    """Walk keeps climbing while ancestors are shims, stops at first non-shim.
+
+    Some installer layouts double-wrap: ``hermes.exe`` (outer) →
+    ``hermes-gateway.exe`` (inner, for some niche commands) → python.
+    Both should be excluded; the shell ancestor above them must not be
+    walked through (and isn't a shim anyway).
+    """
+    scripts_dir = tmp_path
+    outer = scripts_dir / "hermes.exe"
+    inner = scripts_dir / "hermes-gateway.exe"
+    outer.write_bytes(b"")
+    inner.write_bytes(b"")
+    outer_pid = os.getpid() + 10
+    inner_pid = os.getpid() + 11
+    shell_pid = os.getpid() + 12
+    procs = [
+        _make_proc(outer_pid, str(outer), "hermes.exe"),
+        _make_proc(inner_pid, str(inner), "hermes-gateway.exe"),
+        _make_proc(shell_pid, r"C:\\Windows\\System32\\cmd.exe", "cmd.exe"),
+    ]
+    fake = _fake_psutil_with_chain(
+        procs,
+        parent_chain=[
+            (inner_pid, str(inner)),
+            (outer_pid, str(outer)),
+            (shell_pid, r"C:\\Windows\\System32\\cmd.exe"),
+        ],
+    )
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    assert result == []
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_walk_stops_at_first_non_shim_ancestor(_winp, tmp_path):
+    """A non-shim ancestor terminates the walk so deeper shims stay visible.
+
+    Concocted but important contract: if the immediate parent is a shell
+    and there happens to be a ``hermes.exe`` further up the tree (e.g. a
+    user wrapper script), we do NOT skip past the shell to exclude it.
+    Anything beyond the first non-shim ancestor is treated as foreign.
+    """
+    scripts_dir = tmp_path
+    shim = scripts_dir / "hermes.exe"
+    shim.write_bytes(b"")
+    shell_pid = os.getpid() + 20
+    far_hermes_pid = os.getpid() + 21
+    procs = [
+        _make_proc(far_hermes_pid, str(shim), "hermes.exe"),
+    ]
+    fake = _fake_psutil_with_chain(
+        procs,
+        parent_chain=[
+            (shell_pid, r"C:\\Windows\\System32\\cmd.exe"),
+            (far_hermes_pid, str(shim)),
+        ],
+    )
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    # The hermes.exe above the cmd.exe ancestor is NOT excluded — it's a
+    # genuinely separate Hermes process from this invocation's POV.
+    assert result == [(far_hermes_pid, "hermes.exe")]
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_walk_tolerates_psutil_errors(_winp, tmp_path):
+    """An exception in the parent walk must not crash detection.
+
+    psutil routinely raises ``AccessDenied`` / ``NoSuchProcess`` on the
+    PID 0 / PID 4 ancestors on Windows. The walk should degrade
+    gracefully — at worst the launcher gets reported, but the rest of
+    the gate must keep working.
+    """
+    scripts_dir = tmp_path
+    shim = scripts_dir / "hermes.exe"
+    shim.write_bytes(b"")
+    peer_pid = os.getpid() + 30
+
+    class _ExplodingProc:
+        pid = -1
+
+        def parent(self):
+            raise RuntimeError("simulated AccessDenied")
+
+        def exe(self):
+            raise RuntimeError("simulated AccessDenied")
+
+    procs = [_make_proc(peer_pid, str(shim), "hermes.exe")]
+    fake = types.SimpleNamespace(
+        process_iter=lambda attrs: iter(procs),
+        Process=lambda pid=None: _ExplodingProc(),
+        Error=Exception,
+    )
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    # The peer is reported (no special launcher to exclude got found), and
+    # nothing crashed.
+    assert result == [(peer_pid, "hermes.exe")]
+
+
+@patch.object(cli_main, "_is_windows", return_value=True)
+def test_detect_concurrent_walk_is_bounded(_winp, tmp_path):
+    """A pathological parent chain must not loop forever.
+
+    Defensive: walk caps at 16 hops so a buggy psutil/proc table can't
+    hang ``hermes update``.
+    """
+    scripts_dir = tmp_path
+    shim = scripts_dir / "hermes.exe"
+    shim.write_bytes(b"")
+
+    # Build an infinite-looking chain of shim ancestors. The walk must
+    # terminate on its own bound and not stack-overflow / hang.
+    long_chain = [(os.getpid() + 100 + i, str(shim)) for i in range(64)]
+    procs = [_make_proc(pid, exe, "hermes.exe") for pid, exe in long_chain]
+
+    fake = _fake_psutil_with_chain(procs, parent_chain=long_chain)
+    with patch.dict(sys.modules, {"psutil": fake}):
+        result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
+
+    # First 16 ancestors are excluded; the remaining 48 are reported. The
+    # exact split is implementation-detail-coupled to the bound, but the
+    # invariant we care about is: detection terminated AND did not
+    # exclude more than the bound.
+    excluded = len(long_chain) - len(result)
+    assert excluded <= 16 + 1  # +1 for os.getpid()
+    assert len(result) >= len(long_chain) - 16
+
+
 # ---------------------------------------------------------------------------
 # _format_concurrent_instances_message
 # ---------------------------------------------------------------------------

website/docs/getting-started/updating.md

@@ -89,6 +89,8 @@ $ hermes update
 
 Close the listed processes and re-run. If you're sure the concurrent process won't interfere (rare — usually only useful when an antivirus shim is mis-attributed), pass `--force` to skip the check. In that case the updater will still retry the `.exe` rename with exponential backoff and, on stubborn locks, schedule the replacement for next reboot via `MoveFileEx(MOVEFILE_DELAY_UNTIL_REBOOT)` so the update can complete.
 
+The detector deliberately ignores the distlib-generated `Scripts\hermes.exe` console launcher that started the current invocation — that launcher is a parent of the Python process doing the update, not a separate Hermes session. If you do see a PID reported, it really is a different process (Hermes Desktop's backend child, a second terminal's REPL, the gateway, …), not the launcher shim that just printed this message.
+
 Expected output looks like:
 
 ```