From 9fc5b875d1727e7eb9ea30655ff4282ff86b8895 Mon Sep 17 00:00:00 2001 From: Jaiydaan Raj Date: Tue, 28 Jul 2026 13:17:06 +0800 Subject: [PATCH] fix: detach the daemon from its console on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``jarvis start`` spawned the server with ``start_new_session=True``. That is POSIX-only — CPython's Windows ``_execute_child`` names the parameter ``unused_start_new_session`` and ignores it — so on Windows the server inherited the launching console instead of detaching from it. Closing that console, or logging off, therefore delivered CTRL_CLOSE_EVENT to the server. Observed in the wild as the daemon dying overnight, with forrtl: error (200): program aborting due to window-CLOSE event in server.log (the Fortran runtime under NumPy handles the event and aborts). ``jarvis start`` looked like it worked: it printed a PID, wrote the pid file and exited 0, and the server ran for as long as the console stayed open. Registered as a log-on scheduled task, this means the machine comes back up with no backend. Pass DETACHED_PROCESS on Windows so the child gets no console at all, plus CREATE_NEW_PROCESS_GROUP so a Ctrl-C in the parent console cannot reach it. POSIX keeps start_new_session. Verified by attaching to each spawned process with AttachConsole(): start_new_session=True attaches successfully (the child shares a console); DETACHED_PROCESS fails with ERROR_INVALID_HANDLE (no console exists). Co-Authored-By: Claude Opus 5 --- src/openjarvis/cli/daemon_cmd.py | 18 ++++++++-- tests/cli/test_daemon_cmd.py | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/openjarvis/cli/daemon_cmd.py b/src/openjarvis/cli/daemon_cmd.py index e1b25eaf..29d9ea86 100644 --- a/src/openjarvis/cli/daemon_cmd.py +++ b/src/openjarvis/cli/daemon_cmd.py @@ -81,14 +81,28 @@ def start( if agent_name: cmd.extend(["--agent", agent_name]) - # Start as background process + # Start as background process, fully detached from the launching terminal. + # + # ``start_new_session`` is POSIX-only: CPython's Windows ``_execute_child`` + # names the parameter ``unused_start_new_session`` and ignores it. Relying + # on it there leaves the server sharing its parent's console, so closing + # that console — or logging off — delivers CTRL_CLOSE_EVENT and kills the + # daemon. DETACHED_PROCESS gives it no console at all; the new process + # group additionally stops a Ctrl-C in the parent reaching it. DEFAULT_CONFIG_DIR.mkdir(parents=True, exist_ok=True) log_fh = open(_LOG_FILE, "a") # noqa: SIM115 + spawn_kwargs: dict = {} + if sys.platform == "win32": + spawn_kwargs["creationflags"] = ( + subprocess.DETACHED_PROCESS | subprocess.CREATE_NEW_PROCESS_GROUP + ) + else: + spawn_kwargs["start_new_session"] = True proc = subprocess.Popen( cmd, stdout=log_fh, stderr=log_fh, - start_new_session=True, + **spawn_kwargs, ) _write_pid(proc.pid) diff --git a/tests/cli/test_daemon_cmd.py b/tests/cli/test_daemon_cmd.py index 4df3b393..796860d8 100644 --- a/tests/cli/test_daemon_cmd.py +++ b/tests/cli/test_daemon_cmd.py @@ -2,6 +2,7 @@ from __future__ import annotations +import subprocess from pathlib import Path from unittest.mock import MagicMock, patch @@ -79,3 +80,58 @@ class TestDaemonCommands: result = CliRunner().invoke(cli, ["start"]) assert result.exit_code != 0 assert "already running" in result.output + + +class TestDaemonDetachment: + """The spawned server must outlive the console that started it. + + ``start_new_session`` is POSIX-only — CPython's Windows ``_execute_child`` + names the parameter ``unused_start_new_session``. Relying on it there leaves + the server sharing its parent's console, so closing that console (or logging + off) delivers CTRL_CLOSE_EVENT and kills the daemon. + """ + + @staticmethod + def _spawn_kwargs(platform: str) -> dict: + """Return the kwargs ``start`` passes to Popen when spawning the server. + + ``load_config`` is stubbed because it shells out for GPU detection — + patching Popen wholesale would otherwise break config loading before + the spawn is reached. + """ + with ( + patch("openjarvis.cli.daemon_cmd._read_pid", return_value=None), + patch("openjarvis.cli.daemon_cmd._write_pid"), + patch("openjarvis.cli.daemon_cmd.load_config"), + patch("openjarvis.cli.daemon_cmd.sys.platform", platform), + patch("openjarvis.cli.daemon_cmd.subprocess.Popen") as popen, + patch("builtins.open", MagicMock()), + ): + popen.return_value = MagicMock(pid=4321) + result = CliRunner().invoke(cli, ["start"]) + assert result.exit_code == 0, result.output + spawns = [ + c for c in popen.call_args_list if c.args and "serve" in c.args[0] + ] + assert spawns, f"start did not spawn the server: {popen.call_args_list}" + return spawns[-1].kwargs + + def test_windows_spawn_is_detached_from_the_console(self) -> None: + kwargs = self._spawn_kwargs("win32") + flags = kwargs.get("creationflags", 0) + assert flags & subprocess.DETACHED_PROCESS, ( + "server must be spawned with DETACHED_PROCESS on Windows, otherwise " + "closing the launching console kills it" + ) + assert flags & subprocess.CREATE_NEW_PROCESS_GROUP, ( + "server must be in its own process group so Ctrl-C in the parent " + "console does not propagate to it" + ) + assert not kwargs.get("start_new_session"), ( + "start_new_session is ignored on Windows; it must not be relied on" + ) + + def test_posix_spawn_still_uses_start_new_session(self) -> None: + kwargs = self._spawn_kwargs("linux") + assert kwargs.get("start_new_session") is True + assert "creationflags" not in kwargs or kwargs["creationflags"] == 0