Gateway shutdown, orphan cleanup, and systemd restart issues

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #14728 fix(gateway): kill tool subprocesses before adapter disconnect on drain timeout

  merged to main

  files: gateway/run.py, hermes_cli/gateway.py, tests/gateway/test_gateway_shutdown.py, tests/hermes_cli/test_gateway_service.py

  Show the diff

  • gateway/run.py

    @@ -2560,6 +2560,40 @@ async def stop(
                 return
     
             async def _stop_impl() -> None:
    +            def _kill_tool_subprocesses(phase: str) -> None:
    +                """Kill tool subprocesses + tear down terminal envs + browsers.
    +
    +                Called twice in the shutdown path: once eagerly after a
    +                drain timeout forces agent interrupt (so we reclaim bash/
    +                sleep children before systemd TimeoutStopSec escalates to
    +                SIGKILL on the cgroup — #8202), and once as a final
    +                catch-all at the end of _stop_impl() for the graceful
    +                path or anything respawned mid-teardown.
    +
    +                All steps are best-effort; exceptions are swallowed so
    +                one subsystem's failure doesn't block the rest.
    +                """
    +                try:
    +                    from tools.process_registry import process_registry
    +                    _killed = process_registry.kill_all()
    +                    if _killed:
    +                        logger.info(
    +                            "Shutdown (%s): killed %d tool subprocess(es)",
    +                            phase, _killed,
    +                        )
    +                except Exception as _e:
    +                    logger.debug("process_registry.kill_all (%s) error: %s", phase, _e)
    +                try:
    +                    from tools.terminal_tool import cleanup_all_environments
    +                    cleanup_all_environments()
    +                except Exception as _e:
    +                    logger.debug("cleanup_all_environments (%s) error: %s", phase, _e)
    +                try:
    +                    from tools.browser_tool import cleanup_all_browsers
    +                    cleanup_all_browsers()
    +                except Exception as _e:
    +                    logger.debug("cleanup_all_browsers (%s) error: %s", phase, _e)
    +
                 logger.info(
                     "Stopping gateway%s...",
                     " for restart" if self._restart_requested else "",
    @@ -2621,6 +2655,16 @@ async def _stop_impl() -> None:
                         self._update_runtime_status("draining")
                         await asyncio.sleep(0.1)
     
    +                # Kill lingering tool subprocesses NOW, before we spend more
    +                # budget on adapter disconnect / session DB close.  Under
    +                # systemd (TimeoutStopSec bounded by drain_timeout+headroom),
    +                # deferring this to the end of stop() risks systemd escalating
    +                # to SIGKILL on the cgroup first — at which point bash/sleep
    +                # children left behind by an interrupted terminal tool get
    +                # killed by systemd instead of us (issue #8202).  The final
    +                # catch-all cleanup below still runs for the graceful path.
    +                _kill_tool_subprocesses("post-interrupt")
    +
                 if self._restart_requested and self._restart_detached:
                     try:
                         await self._launch_detached_restart_command()
    @@ -2656,22 +2700,13 @@ async def _stop_impl() -> None:
                 self._shutdown_event.set()
     
                 # Global cleanup: kill any remaining tool subprocesses not tied
    -            # to a specific agent (catch-all for zombie prevention).
    -            try:
    -                from tools.process_registry import process_registry
    -                process_registry.kill_all()
    -            except Exception:
    -                pass
    -            try:
    -                from tools.terminal_tool import cleanup_all_environments
    -                cleanup_all_environments()
    -            except Exception:
    -                pass
    -            try:
    -                from tools.browser_tool import cleanup_all_browsers
    -                cleanup_all_browsers()
    -            except Exception:
    -                pass
    +            # to a specific agent (catch-all for zombie prevention). On the
    +            # drain-timeout path we already did this earlier after agent
    +            # interrupt — this second call catches (a) the graceful path
    +            # where drain succeeded without interrupt, and (b) anything
    +            # that got respawned between the earlier call and adapter
    +            # disconnect (defense in depth; safe to call repeatedly).
    +            _kill_tool_subprocesses("final-cleanup")
     
                 # Close SQLite session DBs so the WAL write lock is released.
                 # Without this, --replace and similar restart flows leave the

  • hermes_cli/gateway.py

    @@ -1469,7 +1469,14 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
                 path_entries.append(resolved_node_dir)
     
         common_bin_paths = ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]
    -    restart_timeout = max(60, int(_get_restart_drain_timeout() or 0))
    +    # systemd's TimeoutStopSec must exceed the gateway's drain_timeout so
    +    # there's budget left for post-interrupt cleanup (tool subprocess kill,
    +    # adapter disconnect, session DB close) before systemd escalates to
    +    # SIGKILL on the cgroup — otherwise bash/sleep tool-call children left
    +    # by a force-interrupted agent get reaped by systemd instead of us
    +    # (#8202). 30s of headroom covers the worst case we've observed.
    +    _drain_timeout = int(_get_restart_drain_timeout() or 0)
    +    restart_timeout = max(60, _drain_timeout) + 30
     
         if system:
             username, group_name, home_dir = _system_service_identity(run_as_user)

  • tests/gateway/test_gateway_shutdown.py

    @@ -145,3 +145,86 @@ async def finish_agents():
         # Start, one count-change update, and final update. Allow one extra update
         # if the loop observes the zero-agent state before exiting.
         assert 3 <= runner._update_runtime_status.call_count <= 4
    +
    +
    [redacted:email]
    +async def test_gateway_stop_kills_tool_subprocesses_before_adapter_disconnect_on_timeout(monkeypatch):
    +    """On drain timeout, tool subprocesses must be killed BEFORE adapter
    +    disconnect so systemd's TimeoutStopSec doesn't SIGKILL the cgroup with
    +    bash/sleep children still attached (#8202)."""
    +    runner, adapter = make_restart_runner()
    +    runner._restart_drain_timeout = 0.01  # force timeout path
    +
    +    call_order: list[str] = []
    +
    +    def _fake_kill_all(task_id=None):
    +        call_order.append("kill_all")
    +        return 2
    +
    +    def _fake_cleanup_envs():
    +        call_order.append("cleanup_environments")
    +
    +    def _fake_cleanup_browsers():
    +        call_order.append("cleanup_browsers")
    +
    +    async def _disconnect():
    +        call_order.append("disconnect")
    +
    +    # Patch the module-level names the stop() helper imports lazily.
    +    import tools.process_registry as _pr
    +    import tools.terminal_tool as _tt
    +    import tools.browser_tool as _bt
    +    monkeypatch.setattr(_pr.process_registry, "kill_all", _fake_kill_all)
    +    monkeypatch.setattr(_tt, "cleanup_all_environments", _fake_cleanup_envs)
    +    monkeypatch.setattr(_bt, "cleanup_all_browsers", _fake_cleanup_browsers)
    +
    +    adapter.disconnect = _disconnect
    +
    +    runner._running_agents = {"session": MagicMock()}
    +
    +    with patch("gateway.status.remove_pid_file"), patch("gateway.status.write_runtime_status"):
    +        await runner.stop()
    +
    +    # First kill_all must precede the first disconnect.  (Both the eager
    +    # post-interrupt cleanup and the final catch-all call _kill_tool_
    +    # subprocesses, so we expect kill_all to appear twice total.)
    +    assert "kill_all" in call_order
    +    assert "disconnect" in call_order
    +    first_kill = call_order.index("kill_all")
    +    first_disconnect = call_order.index("disconnect")
    +    assert first_kill < first_disconnect, (
    +        f"Tool subprocesses must be killed before adapter disconnect on "
    +        f"drain timeout, got order: {call_order}"
    +    )
    +    # Defense-in-depth final cleanup still runs.
    +    assert call_order.count("kill_all") >= 2
    +
    +
    [redacted:email]
    +async def test_gateway_stop_kills_tool_subprocesses_on_graceful_path(monkeypatch):
    +    """Graceful shutdown (no drain timeout) must still kill tool subprocesses
    +    exactly once via the final catch-all — regression guard against
    +    accidentally removing that call when refactoring."""
    +    runner, adapter = make_restart_runner()
    +    adapter.disconnect = AsyncMock()
    +
    +    kill_count = 0
    +
    +    def _fake_kill_all(task_id=None):
    +        nonlocal kill_count
    +        kill_count += 1
    +        return 0
    +
    +    import tools.process_registry as _pr
    +    import tools.terminal_tool as _tt
    +    import tools.browser_tool as _bt
    +    monkeypatch.setattr(_pr.process_registry, "kill_all", _fake_kill_all)
    +    monkeypatch.setattr(_tt, "cleanup_all_environments", lambda: None)
    +    monkeypatch.setattr(_bt, "cleanup_all_browsers", lambda: None)
    +
    +    # No running agents → drain returns immediately, no timeout, no eager cleanup.
    +    with patch("gateway.status.remove_pid_file"), patch("gateway.status.write_runtime_status"):
    +        await runner.stop()
    +
    +    # Only the final catch-all fires on the graceful path.
    +    assert kill_count == 1

  • tests/hermes_cli/test_gateway_service.py

    @@ -95,7 +95,10 @@ def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self
             assert "ExecStop=" not in unit
             assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
             assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
    -        assert "TimeoutStopSec=60" in unit
    +        # TimeoutStopSec must exceed the default drain_timeout (60s) so
    +        # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
    +        # (tool subprocess kill, adapter disconnect) runs — issue #8202.
    +        assert "TimeoutStopSec=90" in unit
     
         def test_user_unit_includes_resolved_node_directory_in_path(self, monkeypatch):
             monkeypatch.setattr(gateway_cli.shutil, "which", lambda cmd: "/home/test/.nvm/versions/node/v24.14.0/bin/node" if cmd == "node" else None)
    @@ -111,7 +114,10 @@ def test_system_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(se
             assert "ExecStop=" not in unit
             assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
             assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
    -        assert "TimeoutStopSec=60" in unit
    +        # TimeoutStopSec must exceed the default drain_timeout (60s) so
    +        # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
    +        # (tool subprocess kill, adapter disconnect) runs — issue #8202.
    +        assert "TimeoutStopSec=90" in unit
             assert "WantedBy=multi-user.target" in unit
     
     

• #54090 fix(gateway): reap cgroup orphans via ExecStopPost to unblock systemd restart

  merged to main

  files: gateway/cgroup_cleanup.py, hermes_cli/gateway.py, scripts/release.py, tests/gateway/test_cgroup_cleanup.py, tests/hermes_cli/test_gateway_service.py

  Show the diff

  • gateway/cgroup_cleanup.py

    @@ -0,0 +1,81 @@
    +"""SIGKILL any process left in this systemd unit's cgroup.
    +
    +Runs as ``ExecStopPost=`` so it only fires after the gateway's main process
    +has exited. The gateway already reaps its own tool subprocesses on a clean
    +shutdown; this is the safety net for long-lived helpers it doesn't track
    +(``adb``, platform bridges, etc.) that would otherwise be orphaned in the
    +cgroup and block ``Restart=always`` — issue #37454.
    +
    +We deliberately iterate ``cgroup.procs`` and send per-PID SIGKILLs instead
    +of writing ``1`` to ``cgroup.kill``: the original failure mode in #37454
    +was the kernel returning ``EINVAL`` on the cgroup-wide kill, while per-PID
    +signal delivery uses a separate code path that still works.
    +"""
    +
    +from __future__ import annotations
    +
    +import os
    +import re
    +import signal
    +import sys
    +from pathlib import Path
    +
    +
    +def _own_cgroup_path() -> str | None:
    +    """Return the cgroup v2 path for the calling process, or None."""
    +    try:
    +        text = Path("/proc/self/cgroup").read_text(encoding="utf-8")
    +    except OSError:
    +        return None
    +    match = re.search(r"^0::(.+)$", text, re.MULTILINE)
    +    if not match:
    +        return None
    +    return match.group(1).strip()
    +
    +
    +def _read_cgroup_pids(cgroup_path: str) -> list[int]:
    +    procs_file = Path(f"/sys/fs/cgroup{cgroup_path}/cgroup.procs")
    +    try:
    +        raw = procs_file.read_text(encoding="utf-8")
    +    except OSError:
    +        return []
    +    pids: list[int] = []
    +    for line in raw.splitlines():
    +        line = line.strip()
    +        if not line:
    +            continue
    +        try:
    +            pids.append(int(line))
    +        except ValueError:
    +            continue
    +    return pids
    +
    +
    +def reap_cgroup(cgroup_path: str | None = None) -> int:
    +    """SIGKILL every PID in the cgroup other than the caller. Returns the count killed."""
    +    if cgroup_path is None:
    +        cgroup_path = _own_cgroup_path()
    +    if not cgroup_path:
    +        return 0
    +    own = os.getpid()
    +    killed = 0
    +    for pid in _read_cgroup_pids(cgroup_path):
    +        if pid == own:
    +            continue
    +        try:
    +            os.kill(pid, signal.SIGKILL)  # windows-footgun: ok — Linux-only (reads /proc, /sys/fs/cgroup; runs from a systemd unit)
    +            killed += 1
    +        except ProcessLookupError:
    +            continue
    +        except PermissionError:
    +            continue
    +    return killed
    +
    +
    +def main() -> int:
    +    reap_cgroup()
    +    return 0
    +
    +
    +if __name__ == "__main__":
    +    sys.exit(main())

  • hermes_cli/gateway.py

    @@ -2706,6 +2706,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
     KillMode=mixed
     KillSignal=SIGTERM
     ExecReload=/bin/kill -USR1 $MAINPID
    +ExecStopPost=-{python_path} -m gateway.cgroup_cleanup
     TimeoutStopSec={restart_timeout}
     StandardOutput=journal
     StandardError=journal
    @@ -2739,6 +2740,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
     KillMode=mixed
     KillSignal=SIGTERM
     ExecReload=/bin/kill -USR1 $MAINPID
    +ExecStopPost=-{python_path} -m gateway.cgroup_cleanup
     TimeoutStopSec={restart_timeout}
     StandardOutput=journal
     StandardError=journal

  • scripts/release.py

    @@ -45,6 +45,7 @@
     
     # Auto-extracted from noreply emails + manual overrides
     AUTHOR_MAP = {
    +    "[redacted:email]": "PRATHAMESH75",  # PR #37550 salvage (ExecStopPost cgroup-orphan reaper to unblock systemd restart; #37454)
         "[redacted:email]": "konsisumer",  # PR #19608 salvage (read-modify-write merge in write_credential_pool to preserve concurrently-added credentials; #19566)
         "[redacted:email]": "linyubin",  # PR #50228 salvage (eager fallback on persistent transport timeout/overloaded; #22277)
         "[redacted:email]": "djstunami",  # PR #5316 salvage / co-author (suppress transient check_fn flakes so subagents keep file/terminal tools; #21658 / #5304)

  • tests/gateway/test_cgroup_cleanup.py

    @@ -0,0 +1,100 @@
    +"""Tests for the systemd ExecStopPost cgroup reaper (issue #37454)."""
    +
    +from __future__ import annotations
    +
    +import os
    +import signal
    +from pathlib import Path
    +
    +import pytest
    +
    +from gateway import cgroup_cleanup
    +
    +
    +class TestOwnCgroupPath:
    +    def test_parses_v2_cgroup_path(self, tmp_path, monkeypatch):
    +        proc_self = tmp_path / "cgroup"
    +        proc_self.write_text("0::/user.slice/user-1000.slice/hermes-gateway.service\n")
    +        monkeypatch.setattr(
    +            cgroup_cleanup,
    +            "Path",
    +            lambda p: proc_self if p == "/proc/self/cgroup" else Path(p),
    +        )
    +
    +        assert cgroup_cleanup._own_cgroup_path() == "/user.slice/user-1000.slice/hermes-gateway.service"
    +
    +    def test_returns_none_when_proc_missing(self, monkeypatch):
    +        def _raise(_path):
    +            raise FileNotFoundError
    +
    +        monkeypatch.setattr(cgroup_cleanup.Path, "read_text", lambda self, *a, **k: _raise(self))
    +        assert cgroup_cleanup._own_cgroup_path() is None
    +
    +
    +class TestReapCgroup:
    +    def test_skips_own_pid_and_kills_the_rest(self, tmp_path, monkeypatch):
    +        own = os.getpid()
    +        cgroup_path = "/test.slice/hermes-gateway.service"
    +        procs_file = tmp_path / "cgroup.procs"
    +        procs_file.write_text(f"{own}\n1001\n1002\n\n")
    +
    +        def _fake_path(p):
    +            if p == f"/sys/fs/cgroup{cgroup_path}/cgroup.procs":
    +                return procs_file
    +            return Path(p)
    +
    +        monkeypatch.setattr(cgroup_cleanup, "Path", _fake_path)
    +
    +        killed_pids: list[tuple[int, int]] = []
    +        monkeypatch.setattr(cgroup_cleanup.os, "kill", lambda pid, sig: killed_pids.append((pid, sig)))
    +
    +        count = cgroup_cleanup.reap_cgroup(cgroup_path)
    +
    +        assert count == 2
    +        assert (own, signal.SIGKILL) not in killed_pids
    +        assert (1001, signal.SIGKILL) in killed_pids
    +        assert (1002, signal.SIGKILL) in killed_pids
    +
    +    def test_tolerates_already_exited_pids(self, tmp_path, monkeypatch):
    +        cgroup_path = "/test.slice/hermes-gateway.service"
    +        procs_file = tmp_path / "cgroup.procs"
    +        procs_file.write_text("1001\n1002\n")
    +
    +        monkeypatch.setattr(
    +            cgroup_cleanup,
    +            "Path",
    +            lambda p: procs_file if p.endswith("cgroup.procs") else Path(p),
    +        )
    +
    +        def _kill(pid, _sig):
    +            if pid == 1001:
    +                raise ProcessLookupError
    +            if pid == 1002:
    +                raise PermissionError
    +
    +        monkeypatch.setattr(cgroup_cleanup.os, "kill", _kill)
    +
    +        assert cgroup_cleanup.reap_cgroup(cgroup_path) == 0
    +
    +    def test_noop_when_cgroup_path_unknown(self, monkeypatch):
    +        monkeypatch.setattr(cgroup_cleanup, "_own_cgroup_path", lambda: None)
    +
    +        def _explode(*_a, **_kw):
    +            pytest.fail("os.kill must not be called when cgroup path is unknown")
    +
    +        monkeypatch.setattr(cgroup_cleanup.os, "kill", _explode)
    +        assert cgroup_cleanup.reap_cgroup() == 0
    +
    +    def test_noop_when_procs_file_missing(self, tmp_path, monkeypatch):
    +        cgroup_path = "/missing.slice/hermes-gateway.service"
    +        monkeypatch.setattr(
    +            cgroup_cleanup,
    +            "Path",
    +            lambda p: tmp_path / "does-not-exist" if "cgroup.procs" in p else Path(p),
    +        )
    +
    +        def _explode(*_a, **_kw):
    +            pytest.fail("os.kill must not be called when cgroup.procs is unreadable")
    +
    +        monkeypatch.setattr(cgroup_cleanup.os, "kill", _explode)
    +        assert cgroup_cleanup.reap_cgroup(cgroup_path) == 0

  • tests/hermes_cli/test_gateway_service.py

    @@ -431,6 +431,14 @@ def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self
             # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
             # (tool subprocess kill, adapter disconnect) runs — issue #8202.
             assert self._expected_timeout_stop_sec() in unit
    +        # ExecStopPost reaps any process the gateway didn't clean up itself,
    +        # so long-lived helpers (e.g. adb) can't be left orphaned in the
    +        # cgroup and block Restart=always — issue #37454.
    +        assert "ExecStopPost=" in unit
    +        assert "-m gateway.cgroup_cleanup" in unit
    +        # KillMode=mixed is preserved so the gateway still reaps its own
    +        # tool-call children before systemd SIGKILLs the cgroup — #8202.
    +        assert "KillMode=mixed" in unit
     
         def test_user_unit_includes_resolved_node_directory_in_path(self, monkeypatch):
             monkeypatch.setattr(gateway_cli.shutil, "which", lambda cmd: "/home/test/.nvm/versions/node/v24.14.0/bin/node" if cmd == "node" else None)
    @@ -493,6 +501,14 @@ def test_system_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(se
             # (tool subprocess kill, adapter disconnect) runs — issue #8202.
             assert self._expected_timeout_stop_sec() in unit
             assert "WantedBy=multi-user.target" in unit
    +        # ExecStopPost reaps any process the gateway didn't clean up itself,
    +        # so long-lived helpers (e.g. adb) can't be left orphaned in the
    +        # cgroup and block Restart=always — issue #37454.
    +        assert "ExecStopPost=" in unit
    +        assert "-m gateway.cgroup_cleanup" in unit
    +        # KillMode=mixed is preserved so the gateway still reaps its own
    +        # tool-call children before systemd SIGKILLs the cgroup — #8202.
    +        assert "KillMode=mixed" in unit
     
     
     class TestGatewayStopCleanup:

• #99663 fix(gateway): bound SIGTERM post-interrupt grace (salvage of #65084)

  merged to main

  files: cli-config.yaml.example, gateway/restart.py, gateway/run.py, hermes_cli/config_defaults.py, tests/gateway/restart_test_helpers.py, tests/gateway/test_gateway_shutdown.py, tests/gateway/test_restart_drain.py

• #99675 fix(gateway): run MCP shutdown off-loop with a bounded wait on the SIGTERM path

  merged to main

  files: gateway/run.py, tests/gateway/test_gateway_shutdown.py