fix(embed): harden daemon startup lifecycle

hindsight-all/tests/test_embedded_config.py

@@ -74,6 +74,7 @@ def fake_popen(cmd, env, **kwargs):
         spawned[0] = True
         process = MagicMock()
         process.pid = 12345
+        process.poll.return_value = None  # still running
         return process
 
     with (
@@ -165,9 +166,7 @@ def test_explicit_empty_key_overrides_the_parent_environment(temp_home, monkeypa
     monkeypatch.setenv(LLM_API_KEY, "sk-parent")
     _write_profile(temp_home, "no-auth", 9873)
 
-    env = _daemon_env(
-        HindsightEmbedded(profile="no-auth", llm_provider="lmstudio", llm_api_key="")
-    )
+    env = _daemon_env(HindsightEmbedded(profile="no-auth", llm_provider="lmstudio", llm_api_key=""))
 
     assert env[LLM_API_KEY] == ""
 

hindsight-embed/hindsight_embed/daemon_embed_manager.py

@@ -10,6 +10,7 @@
 import os
 import platform
 import re
+import signal
 import subprocess
 import sys
 import sysconfig
@@ -69,6 +70,7 @@ def _parse_non_negative_int(value: str | None, default: int, name: str) -> int:
 # unpacks and runs initdb on first boot, which takes noticeably longer on cold
 # runners than POSIX.
 DAEMON_STARTUP_TIMEOUT = int(os.getenv("HINDSIGHT_EMBED_DAEMON_STARTUP_TIMEOUT", "180"))
+DAEMON_STARTUP_TERMINATE_TIMEOUT = 10
 DEFAULT_DAEMON_IDLE_TIMEOUT = 0  # 0 = disabled (no auto-exit)
 ENV_DAEMON_LOG_MAX_BYTES = "HINDSIGHT_EMBED_DAEMON_LOG_MAX_BYTES"
 ENV_DAEMON_LOG_BACKUP_COUNT = "HINDSIGHT_EMBED_DAEMON_LOG_BACKUP_COUNT"
@@ -179,6 +181,36 @@ def _detach_popen_kwargs(log_handle: IO[bytes]) -> dict:
     }
 
 
+def _signal_startup_process(process: subprocess.Popen, *, force: bool) -> None:
+    """Signal the daemon child's whole process group on POSIX.
+
+    The child may be the ``uvx`` launcher rather than hindsight-api itself;
+    signalling only its pid can leave the real API running as an orphan.
+    ``_detach_popen_kwargs`` starts it with ``start_new_session``, so its pid is
+    also its process group id. On Windows there is no group to signal, and
+    ``terminate()`` is already an alias for ``kill()``.
+    """
+    if platform.system() == "Windows":
+        process.kill()
+    else:
+        os.killpg(process.pid, signal.SIGKILL if force else signal.SIGTERM)
+
+
+def _terminate_startup_process(process: subprocess.Popen) -> None:
+    """Stop the daemon child owned by a failed startup attempt, escalating to kill."""
+    if process.poll() is not None:
+        return
+    try:
+        _signal_startup_process(process, force=False)
+        try:
+            process.wait(timeout=DAEMON_STARTUP_TERMINATE_TIMEOUT)
+        except subprocess.TimeoutExpired:
+            _signal_startup_process(process, force=True)
+            process.wait(timeout=DAEMON_STARTUP_TERMINATE_TIMEOUT)
+    except ProcessLookupError:
+        return
+
+
 @dataclass(frozen=True)
 class UvTrampoline:
     """A uv venv's Scripts/pythonw.exe, resolved to what it actually launches."""
@@ -853,8 +885,12 @@ def _start_daemon_locked(
         env["HINDSIGHT_API_DAEMON_LOG"] = str(daemon_log)
 
         # Build command
+        # No --daemon: _HINDSIGHT_DAEMON_CHILD above already puts hindsight-api
+        # in daemon mode, and this Popen already detaches it. Passing --daemon
+        # too was redundant, and API versions older than the Popen-based
+        # daemonize() (#1519) fork on it, which can deadlock native libraries
+        # on macOS.
         cmd = self._find_api_command(self._component_version(profile, "HINDSIGHT_EMBED_API_VERSION"), env=env) + [
-            "--daemon",
             "--idle-timeout",
             str(idle_timeout),
             "--port",
@@ -872,7 +908,7 @@ def _start_daemon_locked(
             # Popen dups the fd into the child during spawn, so the parent
             # can close its handle as soon as Popen returns.
             with open(daemon_log, "ab") as daemon_log_handle:
-                subprocess.Popen(cmd, env=env, **_detach_popen_kwargs(daemon_log_handle))
+                daemon_process = subprocess.Popen(cmd, env=env, **_detach_popen_kwargs(daemon_log_handle))
 
             # Wait for daemon to be ready with rich UI
             start_time = time.time()
@@ -889,6 +925,22 @@ def _start_daemon_locked(
                 live.refresh()
 
                 while time.time() - start_time < DAEMON_STARTUP_TIMEOUT:
+                    # Polling keeps going through a missed health probe (below),
+                    # so a child that actually died is detected here instead of
+                    # waiting out the whole startup deadline.
+                    exit_code = daemon_process.poll()
+                    if exit_code is not None:
+                        log_lines.append("")
+                        log_lines.append(f"✗ Daemon exited during initialization (exit code {exit_code})")
+                        log_lines.append(f"See full log: {daemon_log}")
+                        content = Text("\n".join(log_lines), style="dim")
+                        fail_title = f"[bold red]✗ Daemon Failed[/bold red] [dim]({profile} @ :{port})[/dim]"
+                        panel = Panel(content, title=fail_title, border_style="red", padding=(1, 2))
+                        live.update(panel)
+                        live.refresh()
+                        console.print()
+                        return False
+
                     # Tail daemon logs
                     if daemon_log.exists():
                         try:
@@ -937,14 +989,16 @@ def _start_daemon_locked(
                             return True
                         else:
                             log_lines.append("")
-                            log_lines.append("✗ Daemon crashed during initialization")
+                            log_lines.append("Health probe failed after readiness; continuing to wait...")
                             content = Text("\n".join(log_lines), style="dim")
-                            fail_title = f"[bold red]✗ Daemon Failed[/bold red] [dim]({profile} @ :{port})[/dim]"
-                            panel = Panel(content, title=fail_title, border_style="red", padding=(1, 2))
+                            panel = Panel(content, title=title, border_style="cyan", padding=(1, 2))
                             live.update(panel)
                             live.refresh()
-                            console.print()
-                            break
+                            # /health shares the daemon event loop with startup
+                            # backlog processing. A single missed stability probe
+                            # is not evidence that the process exited; keep polling
+                            # within the existing startup deadline.
+                            continue
 
                     # Periodic progress
                     if time.time() - last_check_time > 3:
@@ -972,6 +1026,11 @@ def _start_daemon_locked(
             panel = Panel(content, title=timeout_title, border_style="red", padding=(1, 2))
             console.print(panel)
             console.print()
+            # Before this cleanup, the manager returned at the deadline but left
+            # the detached child initializing. A supervisor retry could then
+            # accumulate overlapping daemons that had not bound the port yet, so
+            # port-based cleanup could not see them.
+            _terminate_startup_process(daemon_process)
             return False
 
         except FileNotFoundError:

hindsight-embed/tests/test_daemon_client.py

@@ -407,7 +407,10 @@ def test_rotation_runs_after_port_clear_and_before_spawn(self, tmp_path, monkeyp
             patch.object(DaemonEmbedManager, "_find_api_command", return_value=["hindsight-api"]),
             patch.object(DaemonEmbedManager, "_component_version", return_value=None),
             patch.object(DaemonEmbedManager, "_register_profile"),
-            patch("subprocess.Popen", side_effect=lambda *_args, **_kwargs: calls.append("spawn")),
+            patch(
+                "subprocess.Popen",
+                side_effect=lambda *_args, **_kwargs: calls.append("spawn") or MagicMock(**{"poll.return_value": None}),
+            ),
             patch("hindsight_embed.daemon_embed_manager.Live"),
         ):
             assert manager._start_daemon_locked({}, "codex", paths) is True

hindsight-embed/tests/test_embed_manager.py

@@ -1,18 +1,118 @@
 """Tests for EmbedManager interface."""
 
 import io
+import signal
 import subprocess
+import sys
+from types import SimpleNamespace
 from unittest.mock import MagicMock, patch
 
+import pytest
+
 from hindsight_embed import get_embed_manager
 from hindsight_embed._http_probe import ProbeResponse
-from hindsight_embed.daemon_embed_manager import DaemonEmbedManager, _detach_popen_kwargs
+from hindsight_embed.daemon_embed_manager import (
+    DaemonEmbedManager,
+    _detach_popen_kwargs,
+    _terminate_startup_process,
+)
 
 
 def _mock_sentence_transformers_present(monkeypatch):
     monkeypatch.setattr("hindsight_embed.daemon_embed_manager.find_spec", lambda name: object())
 
 
+# Process groups and SIGKILL do not exist on Windows.
+_posix_only = pytest.mark.skipif(sys.platform == "win32", reason="POSIX process groups")
+
+
+def _startup_manager(is_running: MagicMock) -> DaemonEmbedManager:
+    manager = DaemonEmbedManager()
+    manager._profile_manager = MagicMock()
+    manager._profile_manager.load_profile_config.return_value = {}
+    manager._clear_port = MagicMock(return_value=True)
+    manager.is_running = is_running
+    manager._component_version = MagicMock(return_value="0.0.0")
+    manager._find_api_command = MagicMock(return_value=["hindsight-api"])
+    return manager
+
+
+def _running_process() -> MagicMock:
+    process = MagicMock(pid=4321)
+    process.poll.return_value = None
+    process.wait.return_value = 0
+    return process
+
+
+@_posix_only
+def test_start_timeout_terminates_spawned_daemon_group(tmp_path, monkeypatch):
+    manager = _startup_manager(MagicMock(return_value=False))
+    process = _running_process()
+    paths = SimpleNamespace(log=tmp_path / "daemon.log", port=9177)
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.DAEMON_STARTUP_TIMEOUT", 0)
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.platform.system", lambda: "Linux")
+    killpg = MagicMock()
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.os.killpg", killpg)
+
+    with patch("hindsight_embed.daemon_embed_manager.subprocess.Popen", return_value=process) as popen:
+        assert manager._start_daemon_locked({}, "test", paths) is False
+
+    assert popen.call_args.args[0] == ["hindsight-api", "--idle-timeout", "0", "--port", "9177"]
+    # The whole group, so a uvx launcher's hindsight-api grandchild goes too.
+    killpg.assert_called_once_with(4321, signal.SIGTERM)
+    process.wait.assert_called_once_with(timeout=10)
+
+
+def test_startup_keeps_polling_after_transient_stability_failure(tmp_path, monkeypatch):
+    manager = _startup_manager(MagicMock(side_effect=[False, True, False, True, True]))
+    process = _running_process()
+    paths = SimpleNamespace(log=tmp_path / "daemon.log", port=9177)
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.time.sleep", lambda _seconds: None)
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.time.time", lambda: 0)
+
+    with patch("hindsight_embed.daemon_embed_manager.subprocess.Popen", return_value=process):
+        assert manager._start_daemon_locked({}, "test", paths) is True
+
+    process.wait.assert_not_called()
+    assert manager.is_running.call_count == 5
+
+
+def test_startup_fails_fast_when_daemon_exits(tmp_path, monkeypatch):
+    manager = _startup_manager(MagicMock(return_value=False))
+    process = _running_process()
+    process.poll.return_value = 1
+    paths = SimpleNamespace(log=tmp_path / "daemon.log", port=9177)
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.time.sleep", MagicMock(side_effect=AssertionError))
+
+    with patch("hindsight_embed.daemon_embed_manager.subprocess.Popen", return_value=process):
+        assert manager._start_daemon_locked({}, "test", paths) is False
+
+    manager.is_running.assert_called_once()  # the pre-spawn check only
+
+
+@_posix_only
+def test_start_timeout_kills_daemon_group_that_ignores_terminate(monkeypatch):
+    process = _running_process()
+    process.wait.side_effect = [subprocess.TimeoutExpired("hindsight-api", 10), 0]
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.platform.system", lambda: "Linux")
+    killpg = MagicMock()
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.os.killpg", killpg)
+
+    _terminate_startup_process(process)
+
+    assert killpg.call_args_list == [((4321, signal.SIGTERM),), ((4321, signal.SIGKILL),)]
+    assert process.wait.call_count == 2
+
+
+def test_start_timeout_kills_daemon_on_windows(monkeypatch):
+    process = _running_process()
+    monkeypatch.setattr("hindsight_embed.daemon_embed_manager.platform.system", lambda: "Windows")
+
+    _terminate_startup_process(process)
+
+    process.kill.assert_called_once_with()
+
+
 def test_sanitize_profile_name_via_db_url():
     """Test profile name sanitization through database URL generation."""
     manager = get_embed_manager()