diff --git a/hindsight-embed/hindsight_embed/daemon_embed_manager.py b/hindsight-embed/hindsight_embed/daemon_embed_manager.py index fc572e9a..c601cea3 100644 --- a/hindsight-embed/hindsight_embed/daemon_embed_manager.py +++ b/hindsight-embed/hindsight_embed/daemon_embed_manager.py @@ -101,6 +101,102 @@ class DaemonEmbedManager(EmbedManager): api_version = os.getenv("HINDSIGHT_EMBED_API_VERSION", __version__) return ["uvx", f"hindsight-api@{api_version}"] + @staticmethod + def _is_port_in_use(port: int) -> bool: + """Check if a port is in use using a socket connection (cross-platform).""" + import socket + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.settimeout(1) + return sock.connect_ex(("127.0.0.1", port)) == 0 + + @staticmethod + def _find_pid_on_port(port: int) -> int | None: + """Find the PID of the process listening on a port.""" + import platform + + try: + if platform.system() == "Windows": + # Use netstat on Windows + result = subprocess.run( + ["netstat", "-ano", "-p", "TCP"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0: + for line in result.stdout.splitlines(): + if f"127.0.0.1:{port}" in line and "LISTENING" in line: + return int(line.strip().split()[-1]) + else: + # Use lsof on macOS/Linux + result = subprocess.run( + ["lsof", "-ti", f":{port}", "-sTCP:LISTEN"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0 and result.stdout.strip(): + return int(result.stdout.strip().split()[0]) + except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError): + pass + return None + + @staticmethod + def _kill_process(pid: int) -> bool: + """Kill a process by PID and wait for it to exit. Returns True if process is gone.""" + import signal + + try: + os.kill(pid, signal.SIGTERM) + for _ in range(50): + time.sleep(0.1) + try: + os.kill(pid, 0) + except OSError: + return True + except OSError: + return True # Already gone + return False + + def _clear_port(self, port: int) -> bool: + """ + Ensure the port is free before starting a daemon. + + If the port is occupied by a hindsight daemon, stop it gracefully. + If occupied by something else, return False. + + Returns: + True if port is free (or was freed), False if occupied by non-hindsight process. + """ + if not self._is_port_in_use(port): + return True + + # Port is occupied — check if it's a hindsight daemon via /health + try: + with httpx.Client(timeout=2) as client: + response = client.get(f"http://127.0.0.1:{port}/health") + if response.status_code != 200: + logger.warning(f"Port {port} is in use by another process") + return False + except Exception: + logger.warning(f"Port {port} is in use by another process") + return False + + # It's a hindsight daemon — find its PID and stop it + pid = self._find_pid_on_port(port) + if pid is None: + logger.warning(f"Port {port} has a hindsight daemon but could not find its PID") + return False + + logger.info(f"Stopping existing daemon on port {port} (PID {pid})") + if self._kill_process(pid): + logger.info(f"Old daemon (PID {pid}) stopped") + return True + + logger.warning(f"Old daemon (PID {pid}) did not stop in time") + return False + def _start_daemon(self, config: dict, profile: str) -> bool: """Start the daemon in background.""" paths = self._profile_manager.resolve_profile_paths(profile) @@ -108,6 +204,11 @@ class DaemonEmbedManager(EmbedManager): daemon_log = paths.log port = paths.port + # Ensure port is free before starting (handles stale daemons from version upgrades) + if not self._clear_port(port): + logger.error(f"Cannot start daemon: port {port} is in use by a non-hindsight process") + return False + # Load profile's .env file and merge with provided config # This fixes issue #305 where profile env vars were ignored profile_config = self._profile_manager.load_profile_config(profile) @@ -502,30 +603,12 @@ class DaemonEmbedManager(EmbedManager): logger.debug(f"UI not running for profile '{profile}'") return True - # Find PID by port - try: - result = subprocess.run( - ["lsof", "-ti", f":{ui_port}", "-sTCP:LISTEN"], - capture_output=True, - text=True, - timeout=5, - ) - if result.returncode == 0 and result.stdout.strip(): - pid = int(result.stdout.strip().split()[0]) - logger.debug(f"Found UI PID {pid} on port {ui_port}") - os.kill(pid, 15) - - # Wait for process to exit - for _ in range(50): - time.sleep(0.1) - try: - os.kill(pid, 0) - except OSError: - break - else: - logger.warning(f"Could not find PID for UI port {ui_port}") - except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError) as e: - logger.warning(f"Could not find/kill UI by port: {e}") + pid = self._find_pid_on_port(ui_port) + if pid is not None: + logger.debug(f"Found UI PID {pid} on port {ui_port}") + self._kill_process(pid) + else: + logger.warning(f"Could not find PID for UI port {ui_port}") # Wait for health check to fail for _ in range(30): @@ -572,32 +655,12 @@ class DaemonEmbedManager(EmbedManager): paths = self._profile_manager.resolve_profile_paths(profile) port = paths.port - # Find PID by port - try: - result = subprocess.run( - ["lsof", "-ti", f":{port}", "-sTCP:LISTEN"], - capture_output=True, - text=True, - timeout=5, - ) - if result.returncode == 0 and result.stdout.strip(): - pid = int(result.stdout.strip().split()[0]) - logger.debug(f"Found daemon PID {pid} on port {port}") - - # Send SIGTERM - os.kill(pid, 15) - - # Wait for process to exit - for _ in range(50): - time.sleep(0.1) - try: - os.kill(pid, 0) - except OSError: - break - else: - logger.warning(f"Could not find PID for port {port}") - except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError) as e: - logger.warning(f"Could not find/kill daemon by port: {e}") + pid = self._find_pid_on_port(port) + if pid is not None: + logger.debug(f"Found daemon PID {pid} on port {port}") + self._kill_process(pid) + else: + logger.warning(f"Could not find PID for port {port}") # Wait for health check to fail for _ in range(30): diff --git a/hindsight-embed/tests/test_daemon_client.py b/hindsight-embed/tests/test_daemon_client.py index f5fd4230..b09ad0ea 100644 --- a/hindsight-embed/tests/test_daemon_client.py +++ b/hindsight-embed/tests/test_daemon_client.py @@ -5,9 +5,11 @@ import subprocess from pathlib import Path from unittest.mock import MagicMock, Mock, patch +import httpx import pytest from hindsight_embed import daemon_client +from hindsight_embed.daemon_embed_manager import DaemonEmbedManager @pytest.fixture def config(): @@ -178,3 +180,93 @@ class TestRunCli: # Verify exit code assert exit_code == 0 + +class TestClearPort: + """Tests for DaemonEmbedManager._clear_port.""" + + def test_port_free(self): + """Port not in use — returns True immediately.""" + manager = DaemonEmbedManager() + with patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=False): + assert manager._clear_port(9555) is True + + def test_port_occupied_by_hindsight_stops_it(self): + """Port occupied by a hindsight daemon — kills it and returns True.""" + manager = DaemonEmbedManager() + with ( + patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True), + patch("httpx.Client") as mock_httpx_cls, + patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=12345), + patch.object(DaemonEmbedManager, "_kill_process", return_value=True), + ): + mock_client = MagicMock() + mock_client.__enter__ = Mock(return_value=mock_client) + mock_client.__exit__ = Mock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_httpx_cls.return_value = mock_client + + assert manager._clear_port(9555) is True + + def test_port_occupied_by_non_hindsight_returns_false(self): + """Port occupied by non-hindsight process — returns False.""" + manager = DaemonEmbedManager() + with ( + patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True), + patch("httpx.Client") as mock_httpx_cls, + ): + mock_client = MagicMock() + mock_client.__enter__ = Mock(return_value=mock_client) + mock_client.__exit__ = Mock(return_value=False) + mock_client.get.side_effect = httpx.ConnectError("Connection refused") + mock_httpx_cls.return_value = mock_client + + assert manager._clear_port(9555) is False + + def test_port_occupied_health_non_200_returns_false(self): + """Port responds but not with 200 — treated as non-hindsight.""" + manager = DaemonEmbedManager() + with ( + patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True), + patch("httpx.Client") as mock_httpx_cls, + ): + mock_client = MagicMock() + mock_client.__enter__ = Mock(return_value=mock_client) + mock_client.__exit__ = Mock(return_value=False) + mock_client.get.return_value = Mock(status_code=404) + mock_httpx_cls.return_value = mock_client + + assert manager._clear_port(9555) is False + + def test_pid_not_found_returns_false(self): + """Hindsight daemon on port but can't find PID — returns False.""" + manager = DaemonEmbedManager() + with ( + patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True), + patch("httpx.Client") as mock_httpx_cls, + patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=None), + ): + mock_client = MagicMock() + mock_client.__enter__ = Mock(return_value=mock_client) + mock_client.__exit__ = Mock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_httpx_cls.return_value = mock_client + + assert manager._clear_port(9555) is False + + def test_kill_fails_returns_false(self): + """Hindsight daemon found but won't die — returns False.""" + manager = DaemonEmbedManager() + with ( + patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True), + patch("httpx.Client") as mock_httpx_cls, + patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=12345), + patch.object(DaemonEmbedManager, "_kill_process", return_value=False), + ): + mock_client = MagicMock() + mock_client.__enter__ = Mock(return_value=mock_client) + mock_client.__exit__ = Mock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_httpx_cls.return_value = mock_client + + assert manager._clear_port(9555) is False +