mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(windows): re-fit env_probe console suppression to the temp-file _run + add no-window tests (#67690 follow-up)
Follow-up to the #67690 salvage (@m4r13y). The PR's tools/env_probe.py hunk was written against the old capture_output=True _run(); #67964/#67999 rewrote _run to temp-file capture on July 20, so that hunk no longer applied — but the rewritten _run still lacked creationflags and kept flashing one console per probe (~5 per kanban worker start) from windowless parents. Re-implement the one-line fix against the current shape: creationflags=windows_hide_flags() on the temp-file subprocess.run, preserving the #67964 grandchild-can't-wedge-the-pipe contract. Also add the tests the PR didn't ship, in tests/test_windows_subprocess_no_window_flags.py: - env_probe._run passes CREATE_NO_WINDOW and keeps temp-file (non-PIPE) stdout/stderr + DEVNULL stdin - lazy_deps uv install / pip --version probe / pip install fallback / ensurepip bootstrap all pass CREATE_NO_WINDOW - suppress_platform_ver_console: POSIX no-op (platform._syscmd_ver untouched, win32_ver() still returns), and simulated-Windows stubbing (echo stub installed, idempotent, never raises)
This commit is contained in:
parent
5c5960d9f9
commit
0b17d4d71e
2 changed files with 158 additions and 0 deletions
|
|
@ -941,3 +941,154 @@ def test_lsp_install_go_hides_console_window(monkeypatch, tmp_path):
|
|||
assert kwargs["creationflags"] == _CREATE_NO_WINDOW
|
||||
assert kwargs["stdin"] == subprocess.DEVNULL
|
||||
assert kwargs["capture_output"] is True
|
||||
|
||||
|
||||
# ── #67690 env probes, lazy installs, platform.win32_ver() (@m4r13y) ───────
|
||||
#
|
||||
# Windowless processes (pythonw gateway + kanban workers) flashed consoles
|
||||
# from three more spawn families: tools/env_probe._run's interpreter/pip
|
||||
# probes, tools/lazy_deps' uv→pip→ensurepip install ladder, and CPython
|
||||
# 3.11/3.12's platform.win32_ver() which shells out `cmd /c ver` with
|
||||
# shell=True and no CREATE_NO_WINDOW. All are hide-only (creationflags);
|
||||
# win32_ver is neutralized by stubbing platform._syscmd_ver so the
|
||||
# documented ValueError fallback reads sys.getwindowsversion() instead.
|
||||
|
||||
|
||||
def test_env_probe_run_hides_console_window(monkeypatch):
|
||||
from tools import env_probe
|
||||
|
||||
captured = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured.append((cmd, kwargs))
|
||||
return _Completed(stdout="", returncode=0)
|
||||
|
||||
monkeypatch.setattr(env_probe, "windows_hide_flags", lambda: _CREATE_NO_WINDOW)
|
||||
monkeypatch.setattr(env_probe.subprocess, "run", fake_run)
|
||||
|
||||
rc, out, err = env_probe._run(["python3", "--version"], timeout=1.0)
|
||||
|
||||
assert rc == 0
|
||||
assert len(captured) == 1, captured
|
||||
cmd, kwargs = captured[0]
|
||||
assert cmd == ["python3", "--version"]
|
||||
assert kwargs["creationflags"] == _CREATE_NO_WINDOW
|
||||
# The temp-file capture contract (#67964) must survive: stdout/stderr are
|
||||
# file objects (not PIPE) so a lingering grandchild can't wedge the probe.
|
||||
assert kwargs["stdout"] is not None and kwargs["stdout"] != subprocess.PIPE
|
||||
assert kwargs["stderr"] is not None and kwargs["stderr"] != subprocess.PIPE
|
||||
assert kwargs["stdin"] == subprocess.DEVNULL
|
||||
|
||||
|
||||
def test_lazy_deps_uv_install_hides_console_window(monkeypatch):
|
||||
from tools import lazy_deps
|
||||
|
||||
captured = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured.append((cmd, kwargs))
|
||||
return _Completed(stdout="installed", returncode=0)
|
||||
|
||||
monkeypatch.delenv(lazy_deps._LAZY_TARGET_ENV, raising=False)
|
||||
monkeypatch.setattr(lazy_deps, "windows_hide_flags", lambda: _CREATE_NO_WINDOW)
|
||||
monkeypatch.setattr(lazy_deps.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(lazy_deps.shutil, "which", lambda name: "/usr/bin/uv" if name == "uv" else None)
|
||||
|
||||
res = lazy_deps._venv_pip_install(("left-pad",))
|
||||
|
||||
assert res.success
|
||||
spawns = _spawns(captured, "pip", "install", "left-pad")
|
||||
assert len(spawns) == 1, captured
|
||||
cmd, kwargs = spawns[0]
|
||||
assert cmd[:3] == ["/usr/bin/uv", "pip", "install"]
|
||||
assert kwargs["creationflags"] == _CREATE_NO_WINDOW
|
||||
assert kwargs["stdin"] == subprocess.DEVNULL
|
||||
|
||||
|
||||
def test_lazy_deps_pip_probe_and_install_hide_console_window(monkeypatch):
|
||||
"""No uv: the pip --version probe and the pip install fallback both hide."""
|
||||
from tools import lazy_deps
|
||||
|
||||
captured = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured.append((cmd, kwargs))
|
||||
return _Completed(stdout="pip 25.0", returncode=0)
|
||||
|
||||
monkeypatch.delenv(lazy_deps._LAZY_TARGET_ENV, raising=False)
|
||||
monkeypatch.setattr(lazy_deps, "windows_hide_flags", lambda: _CREATE_NO_WINDOW)
|
||||
monkeypatch.setattr(lazy_deps.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(lazy_deps.shutil, "which", lambda name: None)
|
||||
|
||||
res = lazy_deps._venv_pip_install(("left-pad",))
|
||||
|
||||
assert res.success
|
||||
probes = _spawns(captured, "-m", "pip", "--version")
|
||||
installs = _spawns(captured, "-m", "pip", "install", "left-pad")
|
||||
assert len(probes) == 1 and len(installs) == 1, captured
|
||||
for _cmd, kwargs in probes + installs:
|
||||
assert kwargs["creationflags"] == _CREATE_NO_WINDOW
|
||||
assert kwargs["stdin"] == subprocess.DEVNULL
|
||||
|
||||
|
||||
def test_lazy_deps_ensurepip_hides_console_window(monkeypatch):
|
||||
"""Failed pip probe: the ensurepip bootstrap spawn hides too."""
|
||||
from tools import lazy_deps
|
||||
|
||||
captured = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured.append((cmd, kwargs))
|
||||
if "--version" in cmd:
|
||||
return _Completed(stdout="", returncode=1) # probe fails → ensurepip
|
||||
return _Completed(stdout="ok", returncode=0)
|
||||
|
||||
monkeypatch.delenv(lazy_deps._LAZY_TARGET_ENV, raising=False)
|
||||
monkeypatch.setattr(lazy_deps, "windows_hide_flags", lambda: _CREATE_NO_WINDOW)
|
||||
monkeypatch.setattr(lazy_deps.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(lazy_deps.shutil, "which", lambda name: None)
|
||||
|
||||
res = lazy_deps._venv_pip_install(("left-pad",))
|
||||
|
||||
assert res.success
|
||||
bootstraps = _spawns(captured, "-m", "ensurepip", "--upgrade")
|
||||
assert len(bootstraps) == 1, captured
|
||||
assert bootstraps[0][1]["creationflags"] == _CREATE_NO_WINDOW
|
||||
|
||||
|
||||
def test_suppress_platform_ver_console_posix_noop(monkeypatch):
|
||||
"""On POSIX the helper must do nothing at all and never raise."""
|
||||
import platform
|
||||
|
||||
from hermes_cli import _subprocess_compat
|
||||
|
||||
monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", False)
|
||||
original = platform._syscmd_ver
|
||||
|
||||
_subprocess_compat.suppress_platform_ver_console()
|
||||
|
||||
assert platform._syscmd_ver is original
|
||||
# win32_ver stays functional (returns empty fields off Windows).
|
||||
assert platform.win32_ver() == ("", "", "", "")
|
||||
|
||||
|
||||
def test_suppress_platform_ver_console_stubs_syscmd_ver(monkeypatch):
|
||||
"""Simulated Windows: _syscmd_ver is replaced by an in-process echo stub
|
||||
so win32_ver() takes its ValueError fallback instead of `cmd /c ver`."""
|
||||
import platform
|
||||
|
||||
from hermes_cli import _subprocess_compat
|
||||
|
||||
monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", True)
|
||||
# Register the original with monkeypatch so it gets restored after.
|
||||
monkeypatch.setattr(platform, "_syscmd_ver", platform._syscmd_ver)
|
||||
|
||||
_subprocess_compat.suppress_platform_ver_console()
|
||||
|
||||
# The stub echoes its inputs — win32_ver() treats the unparseable value
|
||||
# as the documented ValueError path and falls back to
|
||||
# sys.getwindowsversion().platform_version (no subprocess, no window).
|
||||
assert platform._syscmd_ver("s", "r", "v") == ("s", "r", "v")
|
||||
# Idempotent + never raises on repeat calls.
|
||||
_subprocess_compat.suppress_platform_ver_console()
|
||||
assert platform._syscmd_ver() == ("", "", "")
|
||||
|
|
|
|||
|
|
@ -38,6 +38,8 @@ import tempfile
|
|||
import threading
|
||||
from typing import Optional
|
||||
|
||||
from hermes_cli._subprocess_compat import windows_hide_flags
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Module-level cache. The probe result is deterministic for the
|
||||
|
|
@ -105,6 +107,11 @@ def _run(cmd: list[str], timeout: float = 3.0) -> tuple[int, str, str]:
|
|||
timeout=timeout,
|
||||
check=False,
|
||||
stdin=subprocess.DEVNULL,
|
||||
# CREATE_NO_WINDOW (0 on POSIX): the probe runs in
|
||||
# windowless processes (pythonw gateway / kanban workers)
|
||||
# where a console child would otherwise flash a visible
|
||||
# window per probe — ~5 flashes at every worker startup.
|
||||
creationflags=windows_hide_flags(),
|
||||
)
|
||||
except subprocess.TimeoutExpired:
|
||||
return -1, "", "timeout"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue