diff --git a/hindsight-api-slim/hindsight_api/daemon.py b/hindsight-api-slim/hindsight_api/daemon.py index 3238ddf40c..c4458b80b2 100644 --- a/hindsight-api-slim/hindsight_api/daemon.py +++ b/hindsight-api-slim/hindsight_api/daemon.py @@ -83,8 +83,19 @@ def _redirect_stdio_to_log() -> None: sys.stdout.flush() sys.stderr.flush() - with open(os.devnull, "r") as devnull: - os.dup2(devnull.fileno(), sys.stdin.fileno()) + # Detached launchers can leave fd 0 closed and Python's stdin unset. + # If open() reuses fd 0, keep it open: closing it would let the log file + # take that descriptor and turn stdin into a write-only log handle. + stdin_fd = os.open(os.devnull, os.O_RDONLY) + if stdin_fd != 0: + try: + os.dup2(stdin_fd, 0) + finally: + os.close(stdin_fd) + else: + os.set_inheritable(0, True) + if sys.stdin is None or sys.stdin.closed: + sys.stdin = open(0, "r", closefd=False) log_fd = open(daemon_log_path(), "a") os.dup2(log_fd.fileno(), sys.stdout.fileno()) diff --git a/hindsight-api-slim/tests/test_daemon_stdio.py b/hindsight-api-slim/tests/test_daemon_stdio.py new file mode 100644 index 0000000000..b8929445ab --- /dev/null +++ b/hindsight-api-slim/tests/test_daemon_stdio.py @@ -0,0 +1,61 @@ +"""Exercise daemon-child descriptor redirection in disposable Python processes.""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + + +@pytest.mark.skipif(os.name != "posix", reason="Windows daemonize does not redirect stdio") +@pytest.mark.parametrize("stdin_state", ["normal", "none", "closed_fd", "closed_stream", "none_closed_fd"]) +def test_daemon_child_restores_stdin_and_redirects_output(tmp_path: Path, stdin_state: str) -> None: + log_path = tmp_path / "logs" / "daemon.log" + code = """ +import os +import sys +from pathlib import Path +from hindsight_api import daemon + +# Isolate log-path configuration, not descriptor operations or daemon dispatch. +daemon.daemon_log_path = lambda: Path(sys.argv[2]) +state = sys.argv[1] +if state == "closed_stream": + sys.stdin.close() +if state in ("closed_fd", "none_closed_fd"): + os.close(0) +if state in ("none", "none_closed_fd"): + sys.stdin = None + +daemon.daemonize() +assert sys.stdin.read() == "" +assert os.read(0, 1) == b"" +assert os.get_inheritable(0) +print("python stdout", flush=True) +print("python stderr", file=sys.stderr, flush=True) +os.write(1, b"fd stdout\\n") +os.write(2, b"fd stderr\\n") +""" + # Do not load the caller's credentials or .env: config and logs belong to this + # disposable child. Import the checkout, not a separately installed API build. + env = { + "PATH": os.defpath, + "HOME": str(tmp_path), + "PYTHONPATH": str(Path(__file__).resolve().parents[1]), + "HINDSIGHT_API_DAEMON_LOG": str(log_path), + "_HINDSIGHT_DAEMON_CHILD": "1", + } + result = subprocess.run( + [sys.executable, "-c", code, stdin_state, str(log_path)], + env=env, + cwd=tmp_path, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=30, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout == result.stderr == "" + assert log_path.read_text() == "python stdout\npython stderr\nfd stdout\nfd stderr\n" diff --git a/hindsight-integrations/hermes/__init__.py b/hindsight-integrations/hermes/__init__.py index b42ef87e11..068f43dabe 100644 --- a/hindsight-integrations/hermes/__init__.py +++ b/hindsight-integrations/hermes/__init__.py @@ -1125,7 +1125,12 @@ def _log(text: str) -> None: # stop: restarting the daemon now would boot it keyless, which is the # exact outage this guards against. _get_client() above already passed # whatever key WAS available into the in-process client kwargs. - if _load_simple_env(_embedded_profile_env_path(self._config)) != _build_embedded_profile_env(self._config): + profile_env = _load_simple_env(_embedded_profile_env_path(self._config)) + # hindsight-embed persists its resolved listener port in the profile. + # This runtime metadata is not configuration drift: comparing it with + # our generated settings restarts a healthy daemon every session. + profile_env.pop("HINDSIGHT_API_PORT", None) + if profile_env != _build_embedded_profile_env(self._config): if _may_rewrite_profile_env(self._config): _materialize_embedded_profile_env(self._config) if client._manager.is_running(profile): diff --git a/hindsight-integrations/hermes/tests/test_daemon_profile_drift.py b/hindsight-integrations/hermes/tests/test_daemon_profile_drift.py new file mode 100644 index 0000000000..6dc95e9c2a --- /dev/null +++ b/hindsight-integrations/hermes/tests/test_daemon_profile_drift.py @@ -0,0 +1,88 @@ +"""Runtime listener metadata must not restart an otherwise unchanged daemon.""" + +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import MagicMock + +import hindsight_embed.daemon_embed_manager as dem +import pytest + +from conftest import plugin + + +@pytest.fixture +def daemon_client(hermes_env: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[MagicMock]: + """Keep real profile-file I/O, but never start or stop a live daemon.""" + client = MagicMock() + client._manager.is_running.return_value = True + monkeypatch.setattr(plugin.HindsightMemoryProvider, "_get_client", lambda self: client) + monkeypatch.setattr(Path, "home", lambda: hermes_env) + monkeypatch.setenv("HERMES_HOME", str(hermes_env)) + original_console = dem.console + try: + yield client + finally: + if dem.console is not original_console: + dem.console.file.close() + dem.console = original_console + + +@pytest.mark.parametrize("runtime_port", [None, "9177"]) +def test_unchanged_profile_is_not_rewritten_or_restarted(daemon_client: MagicMock, runtime_port: str | None) -> None: + instance = plugin.HindsightMemoryProvider() + instance._config = {"profile": "test-daemon", "llm_provider": "openai", "llm_api_key": "fixture-only"} + profile_path = plugin._materialize_embedded_profile_env(instance._config) + if runtime_port is not None: + with profile_path.open("a", encoding="utf-8") as profile_file: + profile_file.write(f"HINDSIGHT_API_PORT={runtime_port}\n") + original = profile_path.read_bytes() + + instance._daemon_start_worker() + + daemon_client._ensure_started.assert_called_once() + daemon_client._manager.stop.assert_not_called() + assert profile_path.read_bytes() == original + + +@pytest.mark.parametrize( + ("setting", "new_value"), + [ + ("llm_model", "new-model"), + ("llm_provider", "anthropic"), + ("llm_base_url", "https://example.invalid/v1"), + ("llm_api_key", "rotated-fixture-only"), + ("idle_timeout", "600"), + ], +) +def test_real_config_drift_still_rewrites_and_restarts(daemon_client: MagicMock, setting: str, new_value: str) -> None: + instance = plugin.HindsightMemoryProvider() + instance._config = {"profile": "test-daemon", "llm_provider": "openai", "llm_api_key": "fixture-only"} + profile_path = plugin._materialize_embedded_profile_env(instance._config) + with profile_path.open("a", encoding="utf-8") as profile_file: + profile_file.write("HINDSIGHT_API_PORT=9177\n") + instance._config[setting] = new_value + + instance._daemon_start_worker() + + daemon_client._ensure_started.assert_called_once() + daemon_client._manager.stop.assert_called_once_with("test-daemon") + assert plugin._load_simple_env(profile_path) == plugin._build_embedded_profile_env(instance._config) + + +def test_unsafe_rewrite_keeps_existing_key_and_daemon( + daemon_client: MagicMock, monkeypatch: pytest.MonkeyPatch +) -> None: + instance = plugin.HindsightMemoryProvider() + instance._config = {"profile": "test-daemon", "llm_provider": "openai", "llm_api_key": "fixture-only"} + profile_path = plugin._materialize_embedded_profile_env(instance._config) + with profile_path.open("a", encoding="utf-8") as profile_file: + profile_file.write("HINDSIGHT_API_PORT=9177\n") + original = profile_path.read_bytes() + instance._config["llm_model"] = "new-model" + monkeypatch.setattr(plugin, "_may_rewrite_profile_env", lambda config: False) + + instance._daemon_start_worker() + + daemon_client._ensure_started.assert_called_once() + daemon_client._manager.stop.assert_not_called() + assert profile_path.read_bytes() == original