diff --git a/mempalace/hooks_cli.py b/mempalace/hooks_cli.py index 8498103..258f1e0 100644 --- a/mempalace/hooks_cli.py +++ b/mempalace/hooks_cli.py @@ -19,6 +19,27 @@ STATE_DIR = Path.home() / ".mempalace" / "hook_state" PALACE_ROOT = Path.home() / ".mempalace" +def _detached_popen_kwargs() -> dict: + """Kwargs that fully detach a Popen child so the hook process can exit. + + Without these, Windows holds the parent open until the child closes the + inherited stdout/stderr handles — manifesting as "Stop hook hangs" at + session end (#1268). On POSIX the parent can already exit (orphan + reparents to init), but ``start_new_session`` makes the boundary + explicit so signals to the hook don't propagate to the background mine. + """ + kwargs: dict = {"stdin": subprocess.DEVNULL, "close_fds": True} + if os.name == "nt": + flags = 0 + for name in ("DETACHED_PROCESS", "CREATE_NEW_PROCESS_GROUP", "CREATE_BREAKAWAY_FROM_JOB"): + flags |= getattr(subprocess, name, 0) + if flags: + kwargs["creationflags"] = flags + else: + kwargs["start_new_session"] = True + return kwargs + + def _palace_root_exists() -> bool: """User-removable kill-switch. @@ -285,7 +306,7 @@ def _spawn_mine(cmd: list) -> None: STATE_DIR.mkdir(parents=True, exist_ok=True) log_path = STATE_DIR / "hook.log" with open(log_path, "a") as log_f: - proc = subprocess.Popen(cmd, stdout=log_f, stderr=log_f) + proc = subprocess.Popen(cmd, stdout=log_f, stderr=log_f, **_detached_popen_kwargs()) _MINE_PID_FILE.write_text(str(proc.pid)) @@ -350,6 +371,7 @@ def _desktop_toast(body: str, title: str = "MemPalace"): ["notify-send", "--app-name=MemPalace", "--icon=brain", title, body], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + **_detached_popen_kwargs(), ) except OSError: pass @@ -513,6 +535,7 @@ def _ingest_transcript(transcript_path: str): ], stdout=log_f, stderr=log_f, + **_detached_popen_kwargs(), ) _log(f"Transcript ingest started: {path.name}") except OSError: diff --git a/tests/test_hooks_cli.py b/tests/test_hooks_cli.py index 19ecbaf..c4763c9 100644 --- a/tests/test_hooks_cli.py +++ b/tests/test_hooks_cli.py @@ -560,6 +560,78 @@ def test_maybe_auto_ingest_skips_when_mine_running(tmp_path): mock_popen.assert_not_called() +# --- _detached_popen_kwargs --- + + +def test_detached_popen_kwargs_posix(monkeypatch): + """On POSIX, kwargs include start_new_session so the child detaches.""" + from mempalace.hooks_cli import _detached_popen_kwargs + + monkeypatch.setattr("mempalace.hooks_cli.os.name", "posix") + kwargs = _detached_popen_kwargs() + assert kwargs.get("start_new_session") is True + assert kwargs.get("stdin") is subprocess.DEVNULL + assert kwargs.get("close_fds") is True + assert "creationflags" not in kwargs + + +def test_detached_popen_kwargs_windows(monkeypatch): + """On Windows, kwargs include creationflags that fully detach the child. + + Without these, the parent hook hangs at session end on Windows because + the child's inherited stdout/stderr handles keep the parent's exit + blocked (#1268 root cause for the Python hook path). + """ + from mempalace.hooks_cli import _detached_popen_kwargs + + monkeypatch.setattr("mempalace.hooks_cli.os.name", "nt") + # Simulate Windows-only Popen flag constants. Patch on the imported + # subprocess module within hooks_cli so getattr() picks them up. + monkeypatch.setattr( + "mempalace.hooks_cli.subprocess.DETACHED_PROCESS", 0x00000008, raising=False + ) + monkeypatch.setattr( + "mempalace.hooks_cli.subprocess.CREATE_NEW_PROCESS_GROUP", 0x00000200, raising=False + ) + kwargs = _detached_popen_kwargs() + assert kwargs.get("stdin") is subprocess.DEVNULL + assert kwargs.get("close_fds") is True + flags = kwargs.get("creationflags", 0) + assert flags & 0x00000008, "DETACHED_PROCESS must be set" + assert flags & 0x00000200, "CREATE_NEW_PROCESS_GROUP must be set" + + +def test_spawn_mine_uses_detached_kwargs(tmp_path): + """_spawn_mine forwards detached kwargs so the hook can exit cleanly.""" + with patch("mempalace.hooks_cli.STATE_DIR", tmp_path): + with patch("mempalace.hooks_cli._MINE_PID_FILE", tmp_path / "mine.pid"): + with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen: + mock_popen.return_value.pid = 9999 + from mempalace.hooks_cli import _spawn_mine + + _spawn_mine(["mempalace", "mine", "/tmp/x"]) + kwargs = mock_popen.call_args.kwargs + # The exact key set varies by platform; assert on the + # shared invariants that protect against the Windows hang. + assert kwargs.get("stdin") is subprocess.DEVNULL + assert kwargs.get("close_fds") is True + + +def test_ingest_transcript_uses_detached_kwargs(tmp_path): + """_ingest_transcript spawns the convos mine with detach kwargs.""" + transcript = tmp_path / "session.jsonl" + transcript.write_text("x" * 200) # > 100 byte gate + with patch("mempalace.hooks_cli.STATE_DIR", tmp_path): + with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen: + from mempalace.hooks_cli import _ingest_transcript + + _ingest_transcript(str(transcript)) + assert mock_popen.called + kwargs = mock_popen.call_args.kwargs + assert kwargs.get("stdin") is subprocess.DEVNULL + assert kwargs.get("close_fds") is True + + # --- _mine_already_running ---