diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index a949bb70db..7a24bf6d79 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -1707,11 +1707,12 @@ def _reap_unsupervised_gateway_orphans(extra_exclude: set | None = None) -> bool # Windows Task Scheduler is a supervisor too — and the most reliable # signal is the task's own state, not a parent-chain walk. A Scheduled - # Task gateway whose conhost bootstrap has already exited is invisible - # to `_reaper_candidate_is_supervisor_owned` (the parent chain breaks - # before services.exe, fail-open), yet it is alive and supervised. - # Querying the task state catches that case: if HermesGateway is - # Running, the reaper must not touch its process tree (#86098). + # Task gateway whose conhost/VBS bootstrap has already exited is + # invisible to `_reaper_candidate_is_supervisor_owned` (the parent + # chain breaks before services.exe, fail-open), yet it is alive and + # supervised. After that launcher exits the task is typically Ready, + # not Running — treating only Running as supervised still kills the + # detached gateway on every desktop serve start (#86098, #87001). if is_windows(): try: # The install-time task name is profile-aware (Hermes_Gateway / @@ -1722,7 +1723,7 @@ def _reap_unsupervised_gateway_orphans(extra_exclude: set | None = None) -> bool _task_name = get_task_name() except Exception: _task_name = "Hermes_Gateway" - if _windows_scheduled_task_running(_task_name): + if _windows_scheduled_task_supervises(_task_name): return False from gateway.status import _pid_exists, write_planned_stop_marker @@ -1957,31 +1958,29 @@ def is_windows() -> bool: return sys.platform == "win32" -def _windows_scheduled_task_running(task_name: str) -> bool: - """Return True when a Windows scheduled task with ``task_name`` is Running. +# Task Scheduler states that mean "this profile still has an official +# supervisor". Ready is the steady state after the VBS/cmd launcher +# exits and leaves the detached gateway running (#87001). Queued is a +# rare in-between. Disabled / MISSING are not supervisors. +_WINDOWS_TASK_SUPERVISOR_STATES = frozenset({"Running", "Ready", "Queued"}) - Used to treat Task Scheduler as a gateway supervisor on Windows: the - orphan-reap sweep must not kill a gateway that a scheduled task is - actively managing (it writes the planned-stop marker, the gateway exits - cleanly with code 0, and the scheduler never restarts it — silently - killing A2A/messaging on every desktop-app launch). - Best-effort: any failure (missing task, powershell unavailable, timeout) - returns False so the caller falls back to its existing behaviour. +def _windows_scheduled_task_state(task_name: str) -> str | None: + """Return the English ``Get-ScheduledTask`` State, or None on failure. - Implemented with PowerShell (``Get-ScheduledTask``) instead of ``schtasks`` - because the latter localizes its output (a Chinese Windows prints - ``状态: 正在运行``, not ``Status: Running``) and emits the local codepage, - which ``subprocess`` with ``encoding="utf-8"`` silently mangles. The - ``State`` property of ``Get-ScheduledTask`` is an English enum value - (``Running`` / ``Ready`` / ``Disabled``), stable across locales. + Implemented with PowerShell instead of ``schtasks`` because the latter + localizes its output (a Chinese Windows prints ``状态: 正在运行``, not + ``Status: Running``) and emits the local codepage, which ``subprocess`` + with ``encoding="utf-8"`` silently mangles. The ``State`` property is + an English enum value (``Running`` / ``Ready`` / ``Disabled``), stable + across locales. """ if not is_windows(): - return False + return None try: powershell = shutil.which("powershell") or shutil.which("pwsh") if powershell is None: - return False + return None ps_cmd = ( f"$t = Get-ScheduledTask -TaskName '{task_name}' " "-ErrorAction SilentlyContinue; if ($t) { $t.State } else { 'MISSING' }" @@ -1995,11 +1994,40 @@ def _windows_scheduled_task_running(task_name: str) -> bool: timeout=10, ) if result.returncode != 0: - return False + return None state = (result.stdout or "").strip() - return state == "Running" + return state or None except (OSError, subprocess.TimeoutExpired): - return False + return None + + +def _windows_scheduled_task_running(task_name: str) -> bool: + """Return True when a Windows scheduled task with ``task_name`` is Running. + + Narrow helper kept for callers that need the in-flight state. The + orphan-reaper uses ``_windows_scheduled_task_supervises`` instead — + Ready is the normal post-launch state for a detached gateway. + """ + return _windows_scheduled_task_state(task_name) == "Running" + + +def _windows_scheduled_task_supervises(task_name: str) -> bool: + """Return True when Task Scheduler still owns this profile's gateway. + + Used to treat Task Scheduler as a gateway supervisor on Windows: the + orphan-reap sweep must not kill a gateway that a scheduled task + launched and left detached. After the bootstrap exits the task is + Ready, not Running; a Running-only check still writes the planned-stop + marker, the gateway exits cleanly with code 0, and the scheduler never + restarts it — silently killing A2A/messaging on every desktop-app + launch (#86098, #87001). + + Best-effort: any failure (missing task, powershell unavailable, timeout) + returns False so the caller falls back to pidfile / parent-chain + exclusions. + """ + state = _windows_scheduled_task_state(task_name) + return state in _WINDOWS_TASK_SUPERVISOR_STATES def _windows_gateway_should_absorb_console_controls() -> bool: diff --git a/tests/hermes_cli/test_gateway.py b/tests/hermes_cli/test_gateway.py index 4e8cfe5747..70f529c932 100644 --- a/tests/hermes_cli/test_gateway.py +++ b/tests/hermes_cli/test_gateway.py @@ -774,22 +774,21 @@ def test_module_has_logger(): class TestWindowsScheduledTaskSupervisorGuard: - """The reaper must skip entirely when the HermesGateway scheduled task is - Running — Task Scheduler is a supervisor, and the task's own state is the - authoritative signal. + """The reaper must skip when the profile's scheduled task is still a + supervisor — Running *or* Ready. Regression guard: ``_reaper_candidate_is_supervisor_owned`` walks the parent chain up to ``services.exe`` and fails open when the Task-launched bootstrap has already exited (Windows does not reparent, so the chain - breaks). A gateway whose conhost bootstrap exited is then treated as an - orphan: the reaper writes the planned-stop marker, the gateway exits - cleanly with code 0, and the scheduler never restarts it — silently - killing A2A/messaging on every desktop-app launch. Querying the task - state closes that gap without depending on process ancestry. + breaks). After that exit the task is typically Ready, not Running. A + Running-only check then treats the detached gateway as an orphan: the + reaper writes the planned-stop marker, the gateway exits cleanly with + code 0, and the scheduler never restarts it — silently killing + A2A/messaging on every desktop-app launch (#86098, #87001). """ def test_running_task_skips_reap(self, monkeypatch): - """HermesGateway is Running => reaper returns False, kills nothing.""" + """Hermes_Gateway_* is Running => reaper returns False, kills nothing.""" monkeypatch.setattr(gateway, "is_windows", lambda: True) monkeypatch.setattr(gateway, "is_macos", lambda: False) monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) @@ -804,17 +803,17 @@ class TestWindowsScheduledTaskSupervisorGuard: ) queried = [] - def _fake_task_running(name): + def _fake_supervises(name): queried.append(name) return True monkeypatch.setattr( - gateway, "_windows_scheduled_task_running", _fake_task_running + gateway, "_windows_scheduled_task_supervises", _fake_supervises ) # Guard: if the task check were bypassed, these would be reaped. def _boom_find_gateway_pids(exclude_pids=None): - raise AssertionError("must not scan when scheduled task is Running") + raise AssertionError("must not scan when scheduled task supervises") monkeypatch.setattr(gateway, "find_gateway_pids", _boom_find_gateway_pids) killed_pids = [] @@ -826,15 +825,41 @@ class TestWindowsScheduledTaskSupervisorGuard: assert killed_pids == [] assert queried == ["Hermes_Gateway_testprof"] - def test_not_running_task_still_reaps_real_orphan(self, monkeypatch): - """HermesGateway not Running (or missing) => reaper behaves as before - and still reaps a genuine orphan.""" + def test_ready_task_skips_reap(self, monkeypatch): + """Ready is the post-launcher steady state — still supervised (#87001).""" + monkeypatch.setattr(gateway, "is_windows", lambda: True) + monkeypatch.setattr(gateway, "is_macos", lambda: False) + monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) + import hermes_cli.gateway_windows as gateway_windows + + monkeypatch.setattr( + gateway_windows, "get_task_name", lambda: "Hermes_Gateway_testprof" + ) + monkeypatch.setattr( + gateway, "_windows_scheduled_task_state", lambda name: "Ready" + ) + + def _boom_find_gateway_pids(exclude_pids=None): + raise AssertionError("must not scan when scheduled task is Ready") + + monkeypatch.setattr(gateway, "find_gateway_pids", _boom_find_gateway_pids) + killed_pids = [] + monkeypatch.setattr(gateway.os, "kill", lambda pid, sig: killed_pids.append((pid, sig))) + + result = gateway._reap_unsupervised_gateway_orphans() + + assert result is False + assert killed_pids == [] + + def test_disabled_or_missing_task_still_reaps_real_orphan(self, monkeypatch): + """Disabled / missing task => reaper behaves as before and still + reaps a genuine orphan.""" orphan_pid = 99998 monkeypatch.setattr(gateway, "is_windows", lambda: True) monkeypatch.setattr(gateway, "is_macos", lambda: False) monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) - monkeypatch.setattr(gateway, "_windows_scheduled_task_running", lambda name: False) + monkeypatch.setattr(gateway, "_windows_scheduled_task_supervises", lambda name: False) monkeypatch.setattr("gateway.status.get_running_pid", lambda: None) monkeypatch.setattr(gateway, "_get_service_pids", lambda: set()) @@ -866,3 +891,14 @@ class TestWindowsScheduledTaskSupervisorGuard: monkeypatch.setattr(gateway.subprocess, "run", _boom_run) assert gateway._windows_scheduled_task_running("HermesGateway") is False + assert gateway._windows_scheduled_task_supervises("HermesGateway") is False + assert gateway._windows_scheduled_task_state("HermesGateway") is None + + def test_supervises_ready_and_queued_but_not_disabled(self, monkeypatch): + monkeypatch.setattr(gateway, "is_windows", lambda: True) + states = {"Running": True, "Ready": True, "Queued": True, "Disabled": False, "MISSING": False} + + for state, expected in states.items(): + monkeypatch.setattr(gateway, "_windows_scheduled_task_state", lambda name, s=state: s) + assert gateway._windows_scheduled_task_supervises("Hermes_Gateway") is expected, state + assert gateway._windows_scheduled_task_running("Hermes_Gateway") is (state == "Running")