diff --git a/.github/workflows/windows-venv-e2e.yml b/.github/workflows/windows-venv-e2e.yml index cea7ef5f4e..a09b2f2d45 100644 --- a/.github/workflows/windows-venv-e2e.yml +++ b/.github/workflows/windows-venv-e2e.yml @@ -70,3 +70,11 @@ jobs: uv run --no-sync python -m pytest \ tests/gateway/test_telegram_closewait_windows_live.py \ -o addopts= -v -p no:cacheprovider + + - name: Run background-executor spawn parity live E2E (#70716) + shell: bash + run: | + set -uo pipefail + uv run --no-sync python -m pytest \ + tests/tools/test_process_registry_windows_live.py \ + -o addopts= -v -p no:cacheprovider diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 8725a7f507..ae97defcfe 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -2285,6 +2285,77 @@ class TestSystemdCgroupIsolation: assert pr._stop_systemd_unit("hermes-worker-gone.scope") is True + def test_darwin_never_takes_scope_path_even_with_systemd_run_on_path( + self, registry, monkeypatch, _gateway_identity + ): + """macOS no-op guarantee (#70716 cross-platform audit). + + With ``_IS_LINUX = False`` (darwin), the spawn path must be + byte-identical to the legacy path even when a ``systemd-run`` + binary is somehow on PATH and the gateway identity checks pass: + no probe, no wrapping, no unit recorded. + """ + import tools.process_registry as pr + + fake_popen, captured = self._fake_popen_capture() + + monkeypatch.setattr(pr, "_IS_LINUX", False) + monkeypatch.setattr(pr, "_IS_WINDOWS", False) + monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None) + monkeypatch.setattr("tools.process_registry._find_shell", lambda: "/bin/bash") + monkeypatch.setattr( + "gateway.restart.is_gateway_supervisor_process", lambda: True + ) + # If any branch consults the probe or builds a scope argv on darwin, + # fail loudly. + monkeypatch.setattr("shutil.which", lambda name: "/usr/local/bin/systemd-run") + scope_builds = [] + real_build = pr._build_systemd_scope_argv + monkeypatch.setattr( + pr, + "_build_systemd_scope_argv", + lambda *a, **k: scope_builds.append(a) or real_build(*a, **k), + ) + probe_runs = [] + + def fake_probe_run(argv, **kwargs): + probe_runs.append(argv) + return subprocess.CompletedProcess(args=argv, returncode=0) + + monkeypatch.setattr("subprocess.run", fake_probe_run) + + with ( + patch("subprocess.Popen", side_effect=fake_popen), + patch("threading.Thread", return_value=MagicMock()), + patch.object(registry, "_write_checkpoint"), + ): + session = registry.spawn_local("echo hello", cwd="/tmp") + + argv = captured["argv"] + assert argv == ["/bin/bash", "-lic", "set +m; echo hello"], argv + assert captured["start_new_session"] is True + assert session.systemd_unit == "" + assert scope_builds == [], "darwin must never build a systemd scope argv" + assert probe_runs == [], "darwin must never run the systemd-run probe" + + def test_probe_returns_false_off_linux(self, monkeypatch): + """``_systemd_run_user_scope_available`` is False on non-Linux even + when a ``systemd-run`` binary exists on PATH.""" + import tools.process_registry as pr + + monkeypatch.setattr(pr, "_IS_LINUX", False) + monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None) + monkeypatch.setattr("shutil.which", lambda name: "/usr/local/bin/systemd-run") + probe_runs = [] + monkeypatch.setattr( + "subprocess.run", + lambda argv, **kwargs: probe_runs.append(argv) + or subprocess.CompletedProcess(args=argv, returncode=0), + ) + + assert pr._systemd_run_user_scope_available() is False + assert probe_runs == [], "non-Linux must not exec the probe" + class TestNotificationRedaction: """Background-process notification delivery (completion_queue) applies the diff --git a/tests/tools/test_process_registry_windows_live.py b/tests/tools/test_process_registry_windows_live.py new file mode 100644 index 0000000000..a10c2f2370 --- /dev/null +++ b/tests/tools/test_process_registry_windows_live.py @@ -0,0 +1,103 @@ +"""LIVE Windows E2E for background-executor spawn parity (#70716 / PR salvage). + +Runs ONLY on a real Windows host (the on-demand ``windows-venv-e2e.yml`` +lane). The systemd cgroup-isolation feature for local background executors +must be a strict no-op on Windows: jobs spawn exactly as before, output is +captured, exit codes are correct, and no systemd code path is ever reached +— even when the process claims gateway identity. + +These tests drive the REAL ``ProcessRegistry.spawn_local`` pipe path on the +live Windows process table (real Popen, real Git Bash shell, real reader +thread) — no mocked spawn. +""" + +from __future__ import annotations + +import os +import sys +import time + +import pytest + +pytestmark = pytest.mark.skipif( + sys.platform != "win32", reason="live Windows background-executor E2E" +) + + +@pytest.fixture() +def registry(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes-home")) + import tools.process_registry as pr + + reg = pr.ProcessRegistry() + yield reg + for sid in list(reg._running): + try: + reg.kill_process(sid) + except Exception: + pass + + +def _wait_exit(reg, sid, timeout=60): + deadline = time.time() + timeout + while time.time() < deadline: + sess = reg._finished.get(sid) or reg._running.get(sid) + if sess is not None and sess.exited: + return sess + time.sleep(0.2) + raise AssertionError(f"session {sid} did not exit within {timeout}s") + + +class TestWindowsSpawnParity: + def test_background_job_runs_output_and_exit_code_unchanged(self, registry): + """Plain background job: spawned, output captured, exit code correct.""" + session = registry.spawn_local("echo win-live-parity; exit 7") + done = _wait_exit(registry, session.id) + + assert done.exit_code == 7 + assert "win-live-parity" in done.output_buffer + # The systemd scope identity must never be recorded on Windows. + assert done.systemd_unit == "" + + def test_gateway_identity_never_reaches_systemd_path_on_windows( + self, registry, monkeypatch + ): + """Even with full (faked) gateway identity, the Windows spawn takes + the legacy path: no scope argv is built, no probe runs, and the job + behaves exactly as without the identity.""" + import tools.process_registry as pr + + monkeypatch.setenv("_HERMES_GATEWAY", "1") + monkeypatch.setattr( + "gateway.status.get_running_pid", + lambda *, cleanup_stale=False: os.getpid(), + ) + monkeypatch.setattr( + "gateway.restart.is_gateway_supervisor_process", lambda: True + ) + monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None) + + scope_builds = [] + monkeypatch.setattr( + pr, + "_build_systemd_scope_argv", + lambda *a, **k: scope_builds.append(a) or a[0], + ) + + session = registry.spawn_local("echo win-live-gateway; exit 3") + done = _wait_exit(registry, session.id) + + assert done.exit_code == 3 + assert "win-live-gateway" in done.output_buffer + assert done.systemd_unit == "" + assert scope_builds == [], "Windows must never build a systemd scope argv" + # The availability probe must not have flipped to True on Windows. + assert pr._SYSTEMD_SCOPE_AVAILABLE is not True + + def test_kill_process_windows_plain_path(self, registry): + """kill_process on Windows works without any systemd unit cleanup.""" + session = registry.spawn_local("sleep 60") + time.sleep(1.0) + result = registry.kill_process(session.id) + assert result.get("status") in {"killed", "already_exited"} + assert session.systemd_unit == "" diff --git a/tools/process_registry.py b/tools/process_registry.py index 4ef3177fb3..2c785027ac 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -43,6 +43,10 @@ import uuid from pathlib import Path _IS_WINDOWS = platform.system() == "Windows" +# systemd transient scopes exist only on Linux. Gate every scope-path branch +# on this constant (not merely "not Windows") so macOS and other POSIX +# platforms provably never touch systemd code (#70716 cross-platform audit). +_IS_LINUX = platform.system() == "Linux" from tools.environments.local import _find_shell, _resolve_safe_cwd, _sanitize_subprocess_env from hermes_cli._subprocess_compat import windows_hide_flags from dataclasses import dataclass, field @@ -215,7 +219,7 @@ def _systemd_run_user_scope_available() -> bool: return False available = False - if not _IS_WINDOWS: + if _IS_LINUX: try: import shutil @@ -1098,7 +1102,7 @@ class ProcessRegistry: # Wrap the PTY command in a systemd scope so interactive # executors get their own cgroup, same as pipe mode. pty_in_supervised_gateway = ( - not _IS_WINDOWS and _is_supervised_gateway_process() + _IS_LINUX and _is_supervised_gateway_process() ) pty_use_systemd_scope = ( pty_in_supervised_gateway and _systemd_run_user_scope_available() @@ -1176,7 +1180,7 @@ class ProcessRegistry: # cgroup (and the messaging control plane with it). This applies to # both pipe mode and the PTY path above. shell_argv = [user_shell, "-lic", f"set +m; {safe_command}"] - in_supervised_gateway = not _IS_WINDOWS and _is_supervised_gateway_process() + in_supervised_gateway = _IS_LINUX and _is_supervised_gateway_process() use_systemd_scope = ( in_supervised_gateway and _systemd_run_user_scope_available() )