From 97e2540f5179d1bf7dd785e5876edcf401b66d2e Mon Sep 17 00:00:00 2001 From: RightL Date: Wed, 30 Sep 2026 16:39:50 +0800 Subject: [PATCH] Fix CI portability and Windows daemon job handling Respect native Windows job breakaway policies while refusing unsafe kill-on-close launches. Cover supported and restricted jobs with native regression tests. Align DeepSeek profile expectations with the current provider contract and keep executable-file undo fixtures clean on POSIX. --- README.md | 2 +- rightmemory/platform.py | 55 +++++++++- tests/test_config.py | 4 +- tests/test_platform.py | 37 +++++++ tests/test_pursuit_store.py | 3 + tests/test_windows_process_integration.py | 122 ++++++++++++++++------ 6 files changed, 188 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index a9e3205..a06974f 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ Read-only retrieval and approval-gated orchestration remain optional source skil Open the visual map with `rightmemory pursuit`. It starts or reuses the existing Web Studio and opens an automatic local browser session; check the intended root before editing. See [Pursuit Map](docs/PURSUIT_MAP.md) for the editor and its safety boundaries. -Web Studio starts on demand and keeps running after its launching command or browser closes. Use `rightmemory web stop` to stop it; starting it does not register Windows sign-in or boot tasks. On Windows the background process requests separation from the launcher's process job; the host must permit that separation for the server to outlive the host. +Web Studio starts on demand and keeps running after its launching command or browser closes. Use `rightmemory web stop` to stop it; starting it does not register Windows sign-in or boot tasks. On Windows, background processes use explicit or automatic job breakaway when permitted, and may inherit jobs that do not kill processes on close. Startup is refused when the immediate job forbids breakaway and kills processes on close; enclosing host jobs can still impose lifetime limits. To work from a direction, right-click its node or open **More**, choose **Copy context**, and paste the Markdown into an ordinary Codex App task or conversation alongside your request. The copied background comes from the current canonical graph: the selected direction, its direct incoming and outgoing neighbors, their logical heading ancestors, and direct connections. Readable titles and relationships carry the context without internal identifiers or runtime metadata. Copying leaves the map and Git history unchanged. diff --git a/rightmemory/platform.py b/rightmemory/platform.py index 6b6fd72..6f4959c 100644 --- a/rightmemory/platform.py +++ b/rightmemory/platform.py @@ -99,14 +99,65 @@ def detached_process_kwargs() -> dict[str, object]: flags |= getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) flags |= getattr(subprocess, "CREATE_NO_WINDOW", 0) # A hidden process still inherits a terminal/task host's Windows job. - # Ask to leave that job so its kill-on-close cleanup cannot stop a daemon. - flags |= getattr(subprocess, "CREATE_BREAKAWAY_FROM_JOB", 0) + # Explicit breakaway is rejected by jobs that do not allow it, including + # harmless accounting jobs and hosts that already use silent breakaway. + job_flags = _windows_job_limit_flags() + if job_flags is None or job_flags & 0x0800: # BREAKAWAY_OK + flags |= getattr(subprocess, "CREATE_BREAKAWAY_FROM_JOB", 0) + elif not job_flags & 0x1000 and job_flags & 0x2000: + # Neither explicit nor silent breakaway can escape kill-on-close. + # Do not claim a daemon is detached when closing its host will kill it. + raise RuntimeError( + "Windows host job prevents detached processes; " + "start RightMemory from a host that allows job breakaway" + ) kwargs: dict[str, object] = {"close_fds": True} if flags: kwargs["creationflags"] = flags return kwargs +def _windows_job_limit_flags() -> int | None: + import ctypes + from ctypes import wintypes + + class BasicLimits(ctypes.Structure): + _fields_ = [ + ("process_time", ctypes.c_int64), ("job_time", ctypes.c_int64), + ("flags", wintypes.DWORD), ("min_working_set", ctypes.c_size_t), + ("max_working_set", ctypes.c_size_t), ("active_processes", wintypes.DWORD), + ("affinity", ctypes.c_size_t), ("priority", wintypes.DWORD), + ("scheduling", wintypes.DWORD), + ] + + class ExtendedLimits(ctypes.Structure): + _fields_ = [ + ("basic", BasicLimits), ("io_counters", ctypes.c_uint64 * 6), + ("process_memory", ctypes.c_size_t), ("job_memory", ctypes.c_size_t), + ("peak_process_memory", ctypes.c_size_t), ("peak_job_memory", ctypes.c_size_t), + ] + + kernel = ctypes.WinDLL("kernel32", use_last_error=True) + kernel.GetCurrentProcess.argtypes = () + kernel.GetCurrentProcess.restype = wintypes.HANDLE + kernel.IsProcessInJob.argtypes = (wintypes.HANDLE, wintypes.HANDLE, ctypes.POINTER(wintypes.BOOL)) + kernel.IsProcessInJob.restype = wintypes.BOOL + kernel.QueryInformationJobObject.argtypes = ( + wintypes.HANDLE, ctypes.c_int, ctypes.c_void_p, wintypes.DWORD, ctypes.POINTER(wintypes.DWORD), + ) + kernel.QueryInformationJobObject.restype = wintypes.BOOL + in_job = wintypes.BOOL() + if not kernel.IsProcessInJob(kernel.GetCurrentProcess(), None, ctypes.byref(in_job)): + raise ctypes.WinError(ctypes.get_last_error()) + if not in_job.value: + return None + limits = ExtendedLimits() + # NULL queries the immediate job, whose policy controls child breakaway. + if not kernel.QueryInformationJobObject(None, 9, ctypes.byref(limits), ctypes.sizeof(limits), None): + raise ctypes.WinError(ctypes.get_last_error()) + return limits.basic.flags + + def python_module_child_env() -> dict[str, str]: env = os.environ.copy() source_root = _source_checkout_root() diff --git a/tests/test_config.py b/tests/test_config.py index 8958012..617bafe 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1048,7 +1048,9 @@ def test_deepseek_v4_profile_preserves_thinking_tool_loops(self): model = build_model(config) self.assertTrue(model.profile["supports_thinking"]) - self.assertFalse(model.profile["openai_supports_tool_choice_required"]) + self.assertTrue(model.profile["thinking_enabled_by_default"]) + self.assertTrue(model.profile["supports_forced_tool_choice"]) + self.assertFalse(model.profile["supports_forced_tool_choice_with_thinking"]) self.assertEqual(model.profile["openai_chat_thinking_field"], "reasoning_content") self.assertEqual(model.profile["openai_chat_send_back_thinking_parts"], "field") diff --git a/tests/test_platform.py b/tests/test_platform.py index 50a5f71..ef77939 100644 --- a/tests/test_platform.py +++ b/tests/test_platform.py @@ -159,12 +159,49 @@ def test_windows_detached_process_kwargs_use_creation_flags(self): patch.object(rm_platform.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, create=True), patch.object(rm_platform.subprocess, "CREATE_NO_WINDOW", 0x8000000, create=True), patch.object(rm_platform.subprocess, "CREATE_BREAKAWAY_FROM_JOB", 0x1000000, create=True), + patch.object(rm_platform, "_windows_job_limit_flags", return_value=None), ): kwargs = rm_platform.detached_process_kwargs() self.assertTrue(kwargs["close_fds"]) self.assertEqual(kwargs["creationflags"], 0x9000200) + def test_windows_detachment_respects_job_breakaway_policy(self): + for job_flags, expected in ( + (0x2800, 0x9000200), # Kill-on-close with explicit breakaway + (0x3800, 0x9000200), # Both breakaway modes are permitted + (0x3000, 0x8000200), # Kill-on-close with automatic breakaway + (0x0000, 0x8000200), # Accounting job does not kill on close + ): + with ( + self.subTest(job_flags=hex(job_flags)), + patch.object(rm_platform, "IS_WINDOWS", True), + patch.object(rm_platform, "_windows_job_limit_flags", return_value=job_flags), + patch.object(rm_platform.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, create=True), + patch.object(rm_platform.subprocess, "CREATE_NO_WINDOW", 0x8000000, create=True), + patch.object(rm_platform.subprocess, "CREATE_BREAKAWAY_FROM_JOB", 0x1000000, create=True), + ): + self.assertEqual( + rm_platform.detached_process_kwargs(), + {"close_fds": True, "creationflags": expected}, + ) + + def test_windows_detachment_refuses_non_breakaway_kill_on_close_job(self): + with ( + patch.object(rm_platform, "IS_WINDOWS", True), + patch.object(rm_platform, "_windows_job_limit_flags", return_value=0x2000), + ): + with self.assertRaisesRegex(RuntimeError, "host job prevents detached processes"): + rm_platform.detached_process_kwargs() + + def test_windows_detachment_preserves_job_query_errors(self): + with ( + patch.object(rm_platform, "IS_WINDOWS", True), + patch.object(rm_platform, "_windows_job_limit_flags", side_effect=PermissionError("job query failed")), + ): + with self.assertRaisesRegex(PermissionError, "job query failed"): + rm_platform.detached_process_kwargs() + def test_posix_detached_process_starts_a_new_session(self): with patch.object(rm_platform, "IS_WINDOWS", False): self.assertEqual(rm_platform.detached_process_kwargs(), {"start_new_session": True, "close_fds": True}) diff --git a/tests/test_pursuit_store.py b/tests/test_pursuit_store.py index e89b28c..53fa448 100644 --- a/tests/test_pursuit_store.py +++ b/tests/test_pursuit_store.py @@ -716,10 +716,13 @@ def after_publication(supervisor, *args, **kwargs): def test_delete_undo_restores_executable_backing_file_mode(self): self._apply({"type": "create", "parent_id": "alpha-child", "title": "Grandchild"}) backing = "PURSUIT_alpha-child.md" + backing_path = self.root / backing + backing_path.chmod(backing_path.stat().st_mode | 0o111) self._git("update-index", "--chmod=+x", backing) self._git("commit", "-m", "mark backing executable") before = (self.root / backing).read_bytes() self.assertTrue(self._git("ls-tree", "HEAD", "--", backing).startswith("100755 ")) + self.assertEqual(self._git("status", "--porcelain"), "") deleted = self._pending({"type": "delete", "id": "alpha"}) self.store.flush(self.session_id) self.assertEqual(self._git("ls-tree", "HEAD", "--", backing), "") diff --git a/tests/test_windows_process_integration.py b/tests/test_windows_process_integration.py index 50bcf15..29a4a2d 100644 --- a/tests/test_windows_process_integration.py +++ b/tests/test_windows_process_integration.py @@ -40,39 +40,9 @@ def test_web_survives_launcher_exit_and_owning_job_close(self): # A terminal/task host can close a kill-on-close job after its command # returns. Hidden windows and process groups alone do not detach a server. import ctypes - from ctypes import wintypes - - class BasicLimits(ctypes.Structure): - _fields_ = [ - ("process_time", ctypes.c_int64), ("job_time", ctypes.c_int64), - ("flags", wintypes.DWORD), ("min_working_set", ctypes.c_size_t), - ("max_working_set", ctypes.c_size_t), ("active_processes", wintypes.DWORD), - ("affinity", ctypes.c_size_t), ("priority", wintypes.DWORD), - ("scheduling", wintypes.DWORD), - ] - class ExtendedLimits(ctypes.Structure): - _fields_ = [ - ("basic", BasicLimits), ("io_counters", ctypes.c_uint64 * 6), - ("process_memory", ctypes.c_size_t), ("job_memory", ctypes.c_size_t), - ("peak_process_memory", ctypes.c_size_t), ("peak_job_memory", ctypes.c_size_t), - ] - - kernel = ctypes.WinDLL("kernel32", use_last_error=True) - kernel.CreateJobObjectW.argtypes = (ctypes.c_void_p, wintypes.LPCWSTR) - kernel.CreateJobObjectW.restype = wintypes.HANDLE - kernel.SetInformationJobObject.argtypes = (wintypes.HANDLE, ctypes.c_int, ctypes.c_void_p, wintypes.DWORD) - kernel.SetInformationJobObject.restype = wintypes.BOOL - kernel.AssignProcessToJobObject.argtypes = (wintypes.HANDLE, wintypes.HANDLE) - kernel.AssignProcessToJobObject.restype = wintypes.BOOL - kernel.CloseHandle.argtypes = (wintypes.HANDLE,) - kernel.CloseHandle.restype = wintypes.BOOL - job = kernel.CreateJobObjectW(None, None) - self.assertTrue(job, ctypes.get_last_error()) + kernel, job = self._create_job(0x2000 | 0x0800) # KILL_ON_JOB_CLOSE | BREAKAWAY_OK try: - limits = ExtendedLimits() - limits.basic.flags = 0x2000 | 0x0800 # KILL_ON_JOB_CLOSE | BREAKAWAY_OK - self.assertTrue(kernel.SetInformationJobObject(job, 9, ctypes.byref(limits), ctypes.sizeof(limits))) with tempfile.TemporaryDirectory() as tempdir, socket.socket() as reservation: root = Path(tempdir) reservation.bind(("127.0.0.1", 0)) @@ -119,6 +89,56 @@ class ExtendedLimits(ctypes.Structure): if job: kernel.CloseHandle(job) + def test_detached_process_survives_supported_native_job_close(self): + for flags in (0, 0x2800, 0x3000): # Accounting, explicit breakaway, silent breakaway + with self.subTest(job_flags=hex(flags)): + self._check_detached_process_job(flags) + + def test_detached_process_refuses_native_non_breakaway_kill_job(self): + self._check_detached_process_job(0x2000, refuse=True) + + def _check_detached_process_job(self, flags, *, refuse=False): + import ctypes + + kernel, job = self._create_job(flags) + launcher = None + child_pid = None + try: + # The base interpreter gates the actual launch inside the selected + # job, without a virtualenv redirector adding its own job policy. + code = ( + "import subprocess, sys; " + "from rightmemory.platform import detached_process_kwargs; " + "sys.stdin.readline(); " + "p = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)'], " + "stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, " + "**detached_process_kwargs()); print(p.pid, flush=True)" + ) + launcher = subprocess.Popen( + [sys._base_executable, "-c", code], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + env=python_module_child_env(), text=True, creationflags=subprocess.CREATE_NO_WINDOW, + ) + self.assertTrue(kernel.AssignProcessToJobObject(job, int(launcher._handle)), ctypes.get_last_error()) + output, _ = launcher.communicate("start\n", timeout=10) + if refuse: + self.assertNotEqual(launcher.returncode, 0) + self.assertIn("Windows host job prevents detached processes", output) + return + self.assertEqual(launcher.returncode, 0, output) + child_pid = int(output.strip()) + self.assertTrue(kernel.CloseHandle(job)) + job = None + self.assertFalse(self._wait_until(lambda: not process_exists(child_pid), timeout=0.5)) + finally: + self._terminate_if_running(child_pid) + if launcher is not None: + if launcher.poll() is None: + launcher.kill() + launcher.communicate(timeout=5) + if job: + kernel.CloseHandle(job) + def test_managed_watch_restarts_with_new_registered_pid_and_stops(self): with tempfile.TemporaryDirectory() as tempdir: root = Path(tempdir) @@ -237,6 +257,46 @@ def test_web_startup_timeout_reaps_unregistered_redirector_tree(self): process.kill() process.wait(timeout=5) + def _create_job(self, limit_flags): + import ctypes + from ctypes import wintypes + + class BasicLimits(ctypes.Structure): + _fields_ = [ + ("process_time", ctypes.c_int64), ("job_time", ctypes.c_int64), + ("flags", wintypes.DWORD), ("min_working_set", ctypes.c_size_t), + ("max_working_set", ctypes.c_size_t), ("active_processes", wintypes.DWORD), + ("affinity", ctypes.c_size_t), ("priority", wintypes.DWORD), + ("scheduling", wintypes.DWORD), + ] + + class ExtendedLimits(ctypes.Structure): + _fields_ = [ + ("basic", BasicLimits), ("io_counters", ctypes.c_uint64 * 6), + ("process_memory", ctypes.c_size_t), ("job_memory", ctypes.c_size_t), + ("peak_process_memory", ctypes.c_size_t), ("peak_job_memory", ctypes.c_size_t), + ] + + kernel = ctypes.WinDLL("kernel32", use_last_error=True) + kernel.CreateJobObjectW.argtypes = (ctypes.c_void_p, wintypes.LPCWSTR) + kernel.CreateJobObjectW.restype = wintypes.HANDLE + kernel.SetInformationJobObject.argtypes = (wintypes.HANDLE, ctypes.c_int, ctypes.c_void_p, wintypes.DWORD) + kernel.SetInformationJobObject.restype = wintypes.BOOL + kernel.AssignProcessToJobObject.argtypes = (wintypes.HANDLE, wintypes.HANDLE) + kernel.AssignProcessToJobObject.restype = wintypes.BOOL + kernel.CloseHandle.argtypes = (wintypes.HANDLE,) + kernel.CloseHandle.restype = wintypes.BOOL + job = kernel.CreateJobObjectW(None, None) + self.assertTrue(job, ctypes.get_last_error()) + try: + limits = ExtendedLimits() + limits.basic.flags = limit_flags + self.assertTrue(kernel.SetInformationJobObject(job, 9, ctypes.byref(limits), ctypes.sizeof(limits))) + except BaseException: + kernel.CloseHandle(job) + raise + return kernel, job + def _remove_web_log_after_redirector_exit(self, root: Path) -> None: # The registered server can exit just before its venv redirector releases # inherited log handles. Wait for that release before deleting the fixture.