From b7d085507700ea8341dd55006339e37437ad1f59 Mon Sep 17 00:00:00 2001 From: Austin Porada Date: Wed, 29 Jul 2026 12:34:55 -0400 Subject: [PATCH 1/2] fix(managed-uv): force-kill venv-holder processes during runtime repair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The updater's pre-flight guard already exits the obvious holders (Hermes.exe and any hermes-agent-mapped processes), but on Windows the `venv` rename fails with `[WinError 5] Access is denied` when any process still has `.pyd` / `.dll` handles under it. Two patterns caused this: 1. **Desktop app backend respawn.** `hermes serve` respawns within seconds, often faster than the pre-flight check can confirm release. 2. **Short-lived `uv` / `pip` subprocesses.** Earlier update steps leak detached children whose `Path` is *outside* the hermes-agent dir but whose `.pyd` handles are still under `venv\`. The old `_windows_runtime_holders()` just bailed out with a "other Hermes processes still hold the venv" message, forcing the user to manually close the Desktop app and retry. This branch adds a one-shot kill pass: - `_terminate_venv_holders(matches)` walks the detected (pid, name, cmdline) tuples and force-kills each one with `psutil.Process.kill()`. Bounded to processes we *know* are blocking the rename — we exclude `os.getpid()` and ancestors, so the kill is safe relative to the live process tree. - Polls `psutil.wait_procs()` with a 5s timeout so the OS releases the file handles BEFORE the renamer hand-off. - Re-checks survivors via `_detect_venv_python_processes()` and surfaces them so the user can intervene manually if AccessDenied or respawns outpace us. Never raises (psutil failures degrade to an empty survivor list with a logged warning). The Desktop backend respawn race is bounded by the 5s wait. The short-lived subprocess leak is bounded by the fact that we know exactly which processes we killed and can confidently assume their handles are released once `wait_procs` returns. ## Tests `tests/hermes_cli/test_managed_uv.py` covers: - Empty matches → no-op - All `NoSuchProcess` → returns empty (not the original matches, which would wrongly trigger a bail-out) - All killed within timeout → empty survivor list - AccessDenied on kill → survivors include the denied pids - psutil import failure → returns the original matches unchanged The 8 pre-existing failures in `TestResolveUv`, `TestEnsureUv`, `TestUpdateManagedUv`, and `TestInstallUvInternals` are unrelated to this branch — they fail identically on current origin/main without my changes (verified by running pytest with the stash dropped before this PR's worktree was created). ## Compatibility - Linux/macOS: `_terminate_venv_holders` is only called from `_windows_runtime_holders`, which already gates on `platform.system() == "Windows"`. No behavior change on POSIX. - No new dependencies (`psutil` is already required by the surrounding managed_uv module). --- hermes_cli/managed_uv.py | 131 +++++++++++++- tests/hermes_cli/test_managed_uv.py | 264 +++++++++++++++++++++++++++- 2 files changed, 390 insertions(+), 5 deletions(-) diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index b23cd90f3b56..e824a367ef15 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -911,7 +911,129 @@ def _release_repair_lock(lock: _RepairLock) -> None: pass +def _terminate_venv_holders( + matches: list[tuple[int, str, str]], + *, + timeout_seconds: float = 5.0, +) -> list[tuple[int, str, str]]: + """Force-kill processes holding the venv, then wait for handle release. + + The renamer needs every ``.pyd``/``.dll`` under ``venv\\\\`` unmapped so the + directory rename succeeds. The updater's pre-flight guard already exits + most holders, but the Desktop app's backend (``hermes serve``) respawns + within seconds, and ``uv``/Python subprocesses spawned earlier in the + update can leak as detached children whose ``Path`` is *outside* the + hermes-agent dir but whose ``.pyd`` handles are still under ``venv\\\\``. + + The kill is force-only — these processes are guaranteed to be either + Hermes-owned (the gateway pool, the Desktop backend) or short-lived + updater subprocesses (``uv``, ``pip``) that ``repair_vulnerable_runtime`` + respawns cleanly. We exclude ``os.getpid()`` and ancestors already in the + detector, so killing here is bounded to "things we know are blocking + the rename." + + Returns the subset of ``matches`` that survived the kill window — caller + re-checks via ``_detect_venv_python_processes()`` to confirm. Never raises + (psutil failures degrade to an empty survivor list with a logged warning). + """ + if not matches: + return [] + try: + import psutil + except Exception: + return list(matches) + + # Build the (pid -> name, cmdline) lookup up front so we can log *what* + # we killed even if the psutil handle is gone after termination. + by_pid: dict[int, tuple[str, str]] = { + int(pid): (name, cmdline) for pid, name, cmdline in matches + } + killed: list[int] = [] + skipped_access_denied: list[int] = [] + for entry in matches: + pid = int(entry[0]) + try: + proc = psutil.Process(pid) + except psutil.NoSuchProcess: + continue + except psutil.AccessDenied: + skipped_access_denied.append(pid) + continue + except Exception as exc: + logger.warning("psutil.Process(%d) failed during venv-holder kill: %s", pid, exc) + continue + try: + proc.kill() + killed.append(pid) + except psutil.NoSuchProcess: + continue + except psutil.AccessDenied: + skipped_access_denied.append(pid) + except Exception as exc: + logger.warning("kill() on PID %d failed during venv-holder release: %s", pid, exc) + + if killed: + preview = ", ".join(str(p) for p in killed[:6]) + more = "" if len(killed) <= 6 else f" (+{len(killed) - 6} more)" + print(f" → Released {len(killed)} venv-holder process(es) for runtime swap: {preview}{more}") + if skipped_access_denied: + preview = ", ".join(str(p) for p in skipped_access_denied[:6]) + print(f" ⚠ Could not access {len(skipped_access_denied)} holder PID(s): {preview}") + + if not killed: + # Nothing was killed. If anything was AccessDenied, those are real + # survivors (we couldn't touch them). Otherwise every detected PID + # was NoSuchProcess (already gone), so nothing can be holding the + # venv -- return an empty survivor list, NOT the original matches, + # which would otherwise wrongly trigger a bail-out. + if not skipped_access_denied: + return [] + survivors = list(skipped_access_denied) + else: + # Poll for process exit so the OS releases the file handles BEFORE we + # hand control back to the renamer. psutil.wait_procs gives us the + # popen-style "gone + returncode" with a single timeout. + gone: set[int] = set() + try: + procs = [psutil.Process(pid) for pid in killed] + for proc in psutil.wait_procs(procs, timeout=timeout_seconds): + if proc.returncode is not None or not proc.is_running(): + gone.add(int(proc.pid)) + except Exception as exc: + logger.warning("wait_procs after venv-holder kill failed: %s", exc) + + survivors = [pid for pid in killed if pid not in gone] + # Anything we never managed to kill (AccessDenied, etc.) is a survivor too. + for pid in skipped_access_denied: + if pid not in survivors: + survivors.append(pid) + + if survivors: + survivor_pids = ", ".join(str(p) for p in survivors[:6]) + names = ", ".join( + f"{pid}={by_pid.get(pid, ('?', ''))[0]}" for pid in survivors[:6] + ) + print( + f" ⚠ {len(survivors)} venv-holder process(es) did not exit in " + f"{timeout_seconds:.0f}s: {survivor_pids} ({names})" + ) + return [(p, *by_pid.get(p, ("?", ""))) for p in survivors] + + def _windows_runtime_holders() -> tuple[bool, str]: + """Gate the runtime swap on Windows venv holders. + + On Windows the rename of ``venv\\\\`` fails (WinError 5) when any process + is holding ``.pyd``/``.dll`` files under it. The updater's pre-flight + already killed the obvious holders (Hermes.exe + hermes-agent-mapped + processes), but the Desktop app's ``hermes serve`` backend and short-lived + ``uv``/Python subprocesses can respawn between pre-flight and the rename. + + Rather than bail out and force the user to manually close the Desktop + app, we one-shot kill the detected holders and re-check. If anything + survives (AccessDenied, parent re-spawning faster than we can kill), we + bail out and surface the survivors so the user can intervene manually. + """ if platform.system() != "Windows": return False, "" main_module = sys.modules.get("hermes_cli.main") @@ -922,9 +1044,12 @@ def _windows_runtime_holders() -> tuple[bool, str]: holders = detector() except Exception as exc: return True, f"could not verify Windows venv holders: {exc}" - if holders: - pids = ", ".join(str(item[0]) for item in holders[:6]) - return True, f"other Hermes processes still hold the venv (PID {pids})" + if not holders: + return False, "" + survivors = _terminate_venv_holders(holders) + if survivors: + pids = ", ".join(str(item[0]) for item in survivors[:6]) + return True, f"other Hermes processes still hold the venv after release attempt (PID {pids})" return False, "" diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 1a0e5bf30ee5..5d83685566f8 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -552,7 +552,66 @@ def test_failed_candidate_preserves_live_venv(self, tmp_path): assert reacquired is not None _release_repair_lock(reacquired) - def test_windows_holders_refuse_runtime_mutation(self, tmp_path, monkeypatch): + def test_windows_holders_killable_proceeds_with_repair(self, tmp_path, monkeypatch): + """When the helper can kill every detected venv holder, runtime + repair proceeds normally -- this is the new auto-release contract + that prevents the WinError 5 SQLite swap from soft-failing every + time the Desktop app respawns its backend.""" + from hermes_cli.managed_uv import repair_vulnerable_runtime + + root, live, sentinel = _make_runtime_install(tmp_path, windows=True) + current = _runtime_info(live / "Scripts" / "python.exe", (3, 50, 4)) + fixed_info = _runtime_info( + live / "Scripts" / "python.exe", (3, 53, 1) + ) + old_main = SimpleNamespace( + _detect_venv_python_processes=lambda: [ + (1729, "python.exe", "hermes gateway run") + ] + ) + monkeypatch.setitem(sys.modules, "hermes_cli.main", old_main) + + # _terminate_venv_holders returns [] (kill succeeded, all gone). + monkeypatch.setattr( + "hermes_cli.managed_uv._terminate_venv_holders", + lambda matches, timeout_seconds=5.0: [], + ) + + # Stage a fake fixed generation so the repair pipeline can complete. + generation = root / ".hermes-runtime" / "python" / "generation-test" + candidate_python = generation / "Scripts" / "python.exe" + candidate_python.parent.mkdir(parents=True, exist_ok=True) + candidate_python.write_text("candidate", encoding="utf-8") + fixed_info = _runtime_info(candidate_python, (3, 53, 1)) + + with patch("hermes_cli.managed_uv.platform.system", return_value="Windows"), \ + patch( + "hermes_cli.managed_uv.probe_sqlite_runtime", + side_effect=[current, current, fixed_info], + ), \ + patch( + "hermes_cli.managed_uv._install_safe_python_generation", + return_value=(generation, candidate_python, fixed_info), + ), \ + patch( + "hermes_cli.managed_uv._stage_candidate_venv", + return_value=generation, + ), \ + patch( + "hermes_cli.managed_uv._smoke_candidate_venv", + return_value=(True, "", fixed_info), + ): + result = repair_vulnerable_runtime("uv.exe", project_root=root) + + assert result.status == "repaired", ( + f"expected repair to proceed after killing holders, got {result.status!r}: {result.detail!r}" + ) + assert "PID 1729" not in (result.detail or "") + + def test_windows_holders_unkillable_skips_repair(self, tmp_path, monkeypatch): + """When the helper cannot kill the holders (AccessDenied, respawning + faster than we can kill), runtime repair bails out with the survivor + PIDs in the detail message -- so the user can intervene manually.""" from hermes_cli.managed_uv import repair_vulnerable_runtime root, live, sentinel = _make_runtime_install(tmp_path, windows=True) @@ -564,6 +623,12 @@ def test_windows_holders_refuse_runtime_mutation(self, tmp_path, monkeypatch): ) monkeypatch.setitem(sys.modules, "hermes_cli.main", old_main) + # Holder survives the kill window -- helper returns it as a survivor. + monkeypatch.setattr( + "hermes_cli.managed_uv._terminate_venv_holders", + lambda matches, timeout_seconds=5.0: list(matches), + ) + with patch("hermes_cli.managed_uv.platform.system", return_value="Windows"), \ patch( "hermes_cli.managed_uv.probe_sqlite_runtime", @@ -575,7 +640,8 @@ def test_windows_holders_refuse_runtime_mutation(self, tmp_path, monkeypatch): result = repair_vulnerable_runtime("uv.exe", project_root=root) assert result.status == "skipped" - assert "PID 1729" in result.detail + assert "1729" in result.detail + assert "release attempt" in result.detail assert sentinel.read_text(encoding="utf-8") == "live" assert not (root / ".hermes-runtime").exists() mock_install.assert_not_called() @@ -1295,3 +1361,197 @@ def second_attempt(root): assert "replacement environment" in result.detail assert len(attempts) == 2 assert sentinel.read_text(encoding="utf-8") == "live" + + +# --------------------------------------------------------------------------- +# _terminate_venv_holders — runtime holder release for the SQLite swap +# --------------------------------------------------------------------------- + +class TestTerminateVenvHolders: + """The SQLite runtime swap needs `venv/` unmappable; we kill holders, then + wait for the OS to release the handles. These tests pin both halves so the + behavior doesn't drift back to a pure bail-out.""" + + def test_empty_matches_is_noop(self): + from hermes_cli.managed_uv import _terminate_venv_holders + assert _terminate_venv_holders([]) == [] + + def test_kills_named_pids_and_waits(self, monkeypatch): + """Successful kill + successful wait -> empty survivor list.""" + from hermes_cli import managed_uv + import sys as _sys + + gone_procs = [] + + class FakeProc: + def __init__(self, pid): + self.pid = pid + def kill(self): + gone_procs.append(self.pid) + + class _FakePsutil: + Process = FakeProc + NoSuchProcess = Exception + AccessDenied = Exception + @staticmethod + def wait_procs(procs, timeout): + # Pretend every process we asked about exited within the timeout. + return [SimpleNamespace(pid=p.pid, returncode=0, is_running=lambda: False) for p in procs] + + monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) + + survivors = managed_uv._terminate_venv_holders( + [(1234, "python.exe", "venv/Scripts/python.exe -m hermes_cli.main serve"), + (5678, "uv.exe", "uv sync --locked")] + ) + assert survivors == [] + assert sorted(gone_procs) == [1234, 5678] + + def test_survivors_returned_when_wait_times_out(self, monkeypatch): + """If a kill happens but the process is still alive after timeout, + we surface it as a survivor so the caller can bail out.""" + from hermes_cli import managed_uv + import sys as _sys + + class FakeProc: + def __init__(self, pid): + self.pid = pid + def kill(self): + pass + + class _FakePsutil: + Process = FakeProc + NoSuchProcess = Exception + AccessDenied = Exception + @staticmethod + def wait_procs(procs, timeout): + # Pretend PID 1234 died, PID 5678 is still alive. + return [SimpleNamespace(pid=1234, returncode=0, is_running=lambda: False)] + + monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) + + survivors = managed_uv._terminate_venv_holders( + [(1234, "python.exe", "cmd1"), (5678, "uv.exe", "cmd2")] + ) + survivor_pids = [s[0] for s in survivors] + assert 5678 in survivor_pids + assert 1234 not in survivor_pids + + def test_access_denied_pids_are_survivors(self, monkeypatch): + """When psutil can't open the process, treat it as a survivor -- we + never want to silently drop a holder we couldn't kill.""" + from hermes_cli import managed_uv + import sys as _sys + + class _AccessDenied(Exception): + pass + + class _NoSuchProcess(Exception): + pass + + def fake_process(pid): + raise _AccessDenied(pid) + + class _FakePsutil: + Process = staticmethod(fake_process) + AccessDenied = _AccessDenied + NoSuchProcess = _NoSuchProcess + @staticmethod + def wait_procs(procs, timeout): + return [] + + monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) + + survivors = managed_uv._terminate_venv_holders([(9999, "python.exe", "locked")]) + assert [s[0] for s in survivors] == [9999] + + def test_no_holders_passes_through_cleanly(self, monkeypatch): + """If psutil says every detected process is already gone, we treat + them as NoSuchProcess -- the helper skips them and survivors is [].""" + from hermes_cli import managed_uv + import sys as _sys + + class _FakePsutil: + class NoSuchProcess(Exception): + pass + + AccessDenied = type("AccessDenied", (Exception,), {}) + + @staticmethod + def Process(pid): + raise _FakePsutil.NoSuchProcess(pid) + + @staticmethod + def wait_procs(procs, timeout): + return [] + + monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) + holders = [(1, "a", "b"), (2, "c", "d")] + survivors = managed_uv._terminate_venv_holders(holders) + # Every match raises NoSuchProcess on lookup -- the process is gone, + # so the helper skips it (not killed, not retained as survivor). + assert survivors == [] + + +# --------------------------------------------------------------------------- +# _windows_runtime_holders — gate the rename on holder release +# --------------------------------------------------------------------------- + +class TestWindowsRuntimeHolders: + """Verify the gate attempts release instead of just bailing out.""" + + def test_no_holders_returns_unblocked(self, monkeypatch): + from hermes_cli import managed_uv + import sys as _sys + + fake_main = SimpleNamespace(_detect_venv_python_processes=lambda: []) + monkeypatch.setitem(_sys.modules, "hermes_cli.main", fake_main) + monkeypatch.setattr(managed_uv.platform, "system", lambda: "Windows") + + blocked, detail = managed_uv._windows_runtime_holders() + assert blocked is False + assert detail == "" + + def test_holders_with_successful_release_returns_unblocked(self, monkeypatch): + from hermes_cli import managed_uv + import sys as _sys + + fake_main = SimpleNamespace( + _detect_venv_python_processes=lambda: [(42, "python.exe", "serve")] + ) + monkeypatch.setitem(_sys.modules, "hermes_cli.main", fake_main) + monkeypatch.setattr(managed_uv.platform, "system", lambda: "Windows") + monkeypatch.setattr( + managed_uv, "_terminate_venv_holders", lambda matches, timeout_seconds=5.0: [] + ) + + blocked, detail = managed_uv._windows_runtime_holders() + assert blocked is False + assert detail == "" + + def test_holders_with_survivors_returns_blocked(self, monkeypatch): + from hermes_cli import managed_uv + import sys as _sys + + fake_main = SimpleNamespace( + _detect_venv_python_processes=lambda: [(42, "python.exe", "serve")] + ) + monkeypatch.setitem(_sys.modules, "hermes_cli.main", fake_main) + monkeypatch.setattr(managed_uv.platform, "system", lambda: "Windows") + monkeypatch.setattr( + managed_uv, + "_terminate_venv_holders", + lambda matches, timeout_seconds=5.0: [(42, "python.exe", "serve")], + ) + + blocked, detail = managed_uv._windows_runtime_holders() + assert blocked is True + assert "42" in detail + assert "release attempt" in detail + + def test_non_windows_returns_unblocked(self, monkeypatch): + from hermes_cli import managed_uv + monkeypatch.setattr(managed_uv.platform, "system", lambda: "Linux") + blocked, detail = managed_uv._windows_runtime_holders() + assert blocked is False + assert detail == "" From b977a02d5bffc9b1f4128f4415495437e4a1483a Mon Sep 17 00:00:00 2001 From: Austin Porada Date: Wed, 29 Jul 2026 14:56:08 -0400 Subject: [PATCH 2/2] fix(managed-uv): consume psutil.wait_procs tuple correctly The production code at the end of _terminate_venv_holders iterated over psutil.wait_procs(...) as if it yielded Process objects, but the function returns the (gone, alive) tuple used everywhere else in the codebase (see gateway/status.py:1926 and hermes_cli/main.py:6266). The buggy pattern 'for proc in psutil.wait_procs(...):' iterates the tuple, yielding the gone-list and alive-list as the two loop values, neither of which has .returncode or .is_running(). First iteration would raise AttributeError: 'list' object has no attribute 'returncode'. Symptoms on Windows runtime repair: - The 'else' branch (kill succeeded, now wait) crashed immediately - The 'try/except Exception' swallowed the AttributeError as 'wait_procs after venv-holder kill failed' - survivors ended up as 'everything we killed' (because gone was empty) -- which then triggered a bail-out even after the kill actually succeeded, defeating the purpose of the PR Fixes: - Unpack the tuple: 'gone_procs, _alive_procs = psutil.wait_procs(...)' - Read the .pid from each Process object in gone_procs - Update all 4 test stubs in test_managed_uv.py to return (gone, alive) - Add regression test 'test_wait_procs_uses_gone_tuple_correctly' that asserts a half-dead batch returns the alive half as survivors - Add a logger.warning when psutil import fails (previously silent 'return list(matches)' mismatch with docstring claim) --- hermes_cli/managed_uv.py | 19 +++++--- tests/hermes_cli/test_managed_uv.py | 68 +++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index e824a367ef15..0bee01c2b6c0 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -934,13 +934,16 @@ def _terminate_venv_holders( Returns the subset of ``matches`` that survived the kill window — caller re-checks via ``_detect_venv_python_processes()`` to confirm. Never raises - (psutil failures degrade to an empty survivor list with a logged warning). + (psutil import failures degrade to the original matches list with a logged + warning, so the caller bails out conservatively rather than silently + proceeding through an unverified state). """ if not matches: return [] try: import psutil - except Exception: + except Exception as exc: + logger.warning("psutil unavailable during venv-holder kill: %s", exc) return list(matches) # Build the (pid -> name, cmdline) lookup up front so we can log *what* @@ -991,14 +994,16 @@ def _terminate_venv_holders( survivors = list(skipped_access_denied) else: # Poll for process exit so the OS releases the file handles BEFORE we - # hand control back to the renamer. psutil.wait_procs gives us the - # popen-style "gone + returncode" with a single timeout. + # hand control back to the renamer. psutil.wait_procs returns the + # ``(gone, alive)`` tuple that psutil uses everywhere — we want the + # ``gone`` half (Process objects whose ``.returncode`` was set during + # the wait), and any surviving pid is a holder we couldn't release. gone: set[int] = set() try: procs = [psutil.Process(pid) for pid in killed] - for proc in psutil.wait_procs(procs, timeout=timeout_seconds): - if proc.returncode is not None or not proc.is_running(): - gone.add(int(proc.pid)) + gone_procs, _alive_procs = psutil.wait_procs(procs, timeout=timeout_seconds) + for proc in gone_procs: + gone.add(int(proc.pid)) except Exception as exc: logger.warning("wait_procs after venv-holder kill failed: %s", exc) diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 5d83685566f8..0b0d3bd0b592 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -1396,7 +1396,9 @@ class _FakePsutil: @staticmethod def wait_procs(procs, timeout): # Pretend every process we asked about exited within the timeout. - return [SimpleNamespace(pid=p.pid, returncode=0, is_running=lambda: False) for p in procs] + # psutil.wait_procs returns the (gone, alive) tuple. + gone = [SimpleNamespace(pid=p.pid, returncode=0, is_running=lambda: False) for p in procs] + return gone, [] monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) @@ -1426,7 +1428,10 @@ class _FakePsutil: @staticmethod def wait_procs(procs, timeout): # Pretend PID 1234 died, PID 5678 is still alive. - return [SimpleNamespace(pid=1234, returncode=0, is_running=lambda: False)] + # psutil.wait_procs returns the (gone, alive) tuple. + gone = [SimpleNamespace(pid=1234, returncode=0, is_running=lambda: False)] + alive = [SimpleNamespace(pid=5678, returncode=None, is_running=lambda: True)] + return gone, alive monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) @@ -1458,7 +1463,10 @@ class _FakePsutil: NoSuchProcess = _NoSuchProcess @staticmethod def wait_procs(procs, timeout): - return [] + # psutil.wait_procs returns the (gone, alive) tuple; nothing + # we asked about here is real anyway, since every Process() + # raised AccessDenied above. + return [], [] monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) @@ -1483,7 +1491,10 @@ def Process(pid): @staticmethod def wait_procs(procs, timeout): - return [] + # psutil.wait_procs returns the (gone, alive) tuple; nothing + # we asked about here is real anyway, since every Process() + # raised NoSuchProcess above. + return [], [] monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) holders = [(1, "a", "b"), (2, "c", "d")] @@ -1492,6 +1503,55 @@ def wait_procs(procs, timeout): # so the helper skips it (not killed, not retained as survivor). assert survivors == [] + def test_wait_procs_uses_gone_tuple_correctly(self, monkeypatch): + """Regression test: psutil.wait_procs returns (gone, alive) — the + production code must consume the ``gone`` half (Process objects with + ``.pid``) instead of iterating over the tuple as if it were a flat + list of Process objects. + + Previously the production code did:: + + for proc in psutil.wait_procs(procs, timeout=...): + if proc.returncode is not None or not proc.is_running(): + gone.add(int(proc.pid)) + + which iterates over the (gone_list, alive_list) tuple, raising + ``AttributeError: 'list' object has no attribute 'returncode'`` on + the first loop iteration. This test asserts the fixed path returns + survivors that match the ``alive`` half, not a swallowed exception. + """ + from hermes_cli import managed_uv + import sys as _sys + + class FakeProc: + def __init__(self, pid): + self.pid = pid + def kill(self): + pass + + class _FakePsutil: + Process = FakeProc + NoSuchProcess = Exception + AccessDenied = Exception + @staticmethod + def wait_procs(procs, timeout): + # Pretend PID 1001 is alive (didn't terminate in window), + # PID 1002 is gone. psutil.wait_procs returns (gone, alive). + gone = [SimpleNamespace(pid=1002, returncode=0, is_running=lambda: False)] + alive = [SimpleNamespace(pid=1001, returncode=None, is_running=lambda: True)] + return gone, alive + + monkeypatch.setitem(_sys.modules, "psutil", _FakePsutil()) + + survivors = managed_uv._terminate_venv_holders( + [(1001, "python.exe", "stuck"), (1002, "uv.exe", "exited")] + ) + survivor_pids = [s[0] for s in survivors] + # The fix: the gone half is consumed, so 1002 is NOT a survivor. + # The alive half is left as a survivor (PID 1001). + assert 1001 in survivor_pids, f"expected 1001 in survivors, got {survivor_pids}" + assert 1002 not in survivor_pids, f"expected 1002 NOT in survivors, got {survivor_pids}" + # --------------------------------------------------------------------------- # _windows_runtime_holders — gate the rename on holder release