Merge pull request #687 from Curryraj/fix/windows-daemon-detach

fix: detach the daemon from its console on Windows

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)
 

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,80 @@ 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:
+        # These constants are only exported by ``subprocess`` on Windows.
+        # Supply their documented values so the simulated Windows branch is
+        # still exercised by the POSIX test job.
+        detached_process = getattr(subprocess, "DETACHED_PROCESS", 0x00000008)
+        create_new_process_group = getattr(
+            subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200
+        )
+        with (
+            patch.object(
+                subprocess,
+                "DETACHED_PROCESS",
+                detached_process,
+                create=True,
+            ),
+            patch.object(
+                subprocess,
+                "CREATE_NEW_PROCESS_GROUP",
+                create_new_process_group,
+                create=True,
+            ),
+        ):
+            kwargs = self._spawn_kwargs("win32")
+
+        flags = kwargs.get("creationflags", 0)
+        assert flags & detached_process, (
+            "server must be spawned with DETACHED_PROCESS on Windows, otherwise "
+            "closing the launching console kills it"
+        )
+        assert flags & 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

tests/server/test_api_routes.py

@@ -37,16 +37,22 @@ class TestAgentRoutes:
 
 
 class TestMemoryRoutes:
+    # 503 is the documented response when the native ``openjarvis_rust``
+    # extension is absent from the venv (see TestMemoryRustMissing below).
+    # These tests are only asserting "the route is wired up", so a backend
+    # that cannot be built is tolerated the same way a 500 is.
+    _BACKEND_OPTIONAL = (200, 500, 503)
+
     def test_search(self):
         client = TestClient(_make_app())
         resp = client.post("/v1/memory/search", json={"query": "test"})
         # May fail if SQLite not set up, that's ok
-        assert resp.status_code in (200, 500)
+        assert resp.status_code in self._BACKEND_OPTIONAL
 
     def test_stats(self):
         client = TestClient(_make_app())
         resp = client.get("/v1/memory/stats")
-        assert resp.status_code in (200, 500)
+        assert resp.status_code in self._BACKEND_OPTIONAL
 
 
 class TestMemoryRustMissing: