diff --git a/hermes_cli/local_runtime/endpoint.py b/hermes_cli/local_runtime/endpoint.py index 365890a36a..de0d67d801 100644 --- a/hermes_cli/local_runtime/endpoint.py +++ b/hermes_cli/local_runtime/endpoint.py @@ -32,7 +32,12 @@ def _pid_alive(pid: int) -> bool: def _state_endpoint() -> dict | None: - from hermes_cli.local_runtime.recovery import is_modern, read_state, recorded_process + from hermes_cli.local_runtime.recovery import ( + is_modern, + legacy_recorded_process, + read_state, + recorded_process, + ) state = read_state() base_url = state.get("base_url", "") @@ -42,12 +47,7 @@ def _state_endpoint() -> dict | None: if recorded_process(state) is None: return None else: - # Preserve the legacy endpoint shape, with malformed PID values rejected. - try: - pid = state.get("pid") - if isinstance(pid, bool) or not _pid_alive(int(pid or 0)): - return None - except (TypeError, ValueError, OverflowError): + if legacy_recorded_process(state) is None: return None return {"base_url": base_url, "api_key": state.get("api_key", "")} diff --git a/hermes_cli/local_runtime/recovery.py b/hermes_cli/local_runtime/recovery.py index 176d2ae6de..fd8f1926e3 100644 --- a/hermes_cli/local_runtime/recovery.py +++ b/hermes_cli/local_runtime/recovery.py @@ -81,14 +81,19 @@ def _owner_is_dead(state: dict) -> bool: return False -def _legacy_orphan_process(state: dict): - """Older state lacks birth times: require the exact installed binary and launch arguments.""" +def legacy_recorded_process(state: dict): + """Match an older record only to the managed binary and its exact launch arguments. + + Legacy records lack process birth identity. A live PID alone is not evidence because the OS + can reuse it after llama-server exits. The executable location, state-file age, endpoint key, + and launch arguments together keep old records usable without adopting an unrelated process. + """ from urllib.parse import urlsplit from hermes_cli.local_runtime.bootstrap import models_dir from hermes_cli.local_runtime.supervisor import state_path # A damaged new record must not fall back to weaker legacy evidence. - if os.name != "nt" or is_modern(state): + if is_modern(state): return None try: if not _valid_pid(state["pid"]): @@ -96,10 +101,8 @@ def _legacy_orphan_process(state: dict): proc = psutil.Process(state["pid"]) root = state_path().parent exe = Path(proc.exe()) - relative = exe.relative_to(root) - if len(relative.parts) != 3 or exe.name.lower() != "llama-server.exe": - return None - if proc.parent() is not None or proc.ppid() <= 0: + exe.relative_to(root) + if exe.name.lower() not in ("llama-server", "llama-server.exe"): return None if proc.create_time() > state_path().stat().st_mtime: return None # the PID was reused after this record was written @@ -126,6 +129,26 @@ def _legacy_orphan_process(state: dict): return None +def _legacy_orphan_process(state: dict): + """Windows-only legacy stop recovery after the process identity is established.""" + from hermes_cli.local_runtime.supervisor import state_path + + if os.name != "nt": + return None + proc = legacy_recorded_process(state) + if proc is None: + return None + try: + relative = Path(proc.exe()).relative_to(state_path().parent) + if len(relative.parts) != 3 or relative.name.lower() != "llama-server.exe": + return None + if proc.parent() is not None or proc.ppid() <= 0: + return None + return proc + except (OSError, psutil.Error): + return None + + def stop_recorded_orphan() -> bool: """Explicit user stop only. Refuse uncertain identity or a living recorded owner.""" from hermes_cli.local_runtime.supervisor import LlamaServerSupervisor, state_path diff --git a/tests/hermes_cli/test_local_runtime.py b/tests/hermes_cli/test_local_runtime.py index 0b8600185e..7a966a9ab1 100644 --- a/tests/hermes_cli/test_local_runtime.py +++ b/tests/hermes_cli/test_local_runtime.py @@ -21,6 +21,25 @@ from hermes_cli.local_runtime.binaries import select_backend from hermes_cli.local_runtime.detect import DetectedServer, probe_port +def _write_current_process_state(path: Path, *, base_url: str, api_key: str) -> None: + """Publish a modern state record for the process hosting the test stub.""" + import psutil + + proc = psutil.Process() + parent = proc.parent() + assert parent is not None + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({ + "base_url": base_url, + "api_key": api_key, + "pid": proc.pid, + "create_time": proc.create_time(), + "executable": proc.exe(), + "owner_pid": parent.pid, + "owner_create_time": parent.create_time(), + }), encoding="utf-8") + + # ── stub llama-server ──────────────────────────────────────── @@ -280,13 +299,8 @@ def test_llamacpp_endpoint_resolution_prefers_managed(tmp_path, monkeypatch, stu from hermes_cli.local_runtime import endpoint as ep from hermes_cli.local_runtime.supervisor import state_path - state_path().parent.mkdir(parents=True, exist_ok=True) - state_path().write_text(json.dumps({ - # A LIVE pid: the ownership guard treats health-200 + dead recorded - # pid as a foreign server on our stable port (scratch-profile - # collision), so claiming this test process models "our server". - "base_url": f"http://127.0.0.1:{port}/v1", "api_key": "sk-managed", "pid": os.getpid(), - }), encoding="utf-8") + _write_current_process_state( + state_path(), base_url=f"http://127.0.0.1:{port}/v1", api_key="sk-managed") resolved = ep.resolve_llamacpp_endpoint() assert resolved == {"base_url": f"http://127.0.0.1:{port}/v1", "api_key": "sk-managed"} @@ -371,7 +385,8 @@ def test_llamacpp_endpoint_starting_server_resolves(tmp_path, monkeypatch): "base_url": f"http://127.0.0.1:{not_listening}/v1", "api_key": "sk-starting", "pid": 4242, }), encoding="utf-8") - monkeypatch.setattr(ep, "_pid_alive", lambda pid: True) + monkeypatch.setattr( + "hermes_cli.local_runtime.recovery.legacy_recorded_process", lambda state: object()) resolved = ep.resolve_llamacpp_endpoint() assert resolved is not None assert resolved["api_key"] == "sk-starting" @@ -392,7 +407,8 @@ def test_llamacpp_endpoint_waits_for_boot_in_flight(tmp_path, monkeypatch): # Boot is in flight: runtime enabled + binary installed. monkeypatch.setattr(ep, "_boot_in_flight", lambda config: True) - monkeypatch.setattr(ep, "_pid_alive", lambda pid: True) + monkeypatch.setattr( + "hermes_cli.local_runtime.recovery.legacy_recorded_process", lambda state: object()) # Nothing detected externally. monkeypatch.setattr("hermes_cli.local_runtime.detect.DEFAULT_PROBE_PORTS", ()) @@ -430,7 +446,8 @@ def test_resolution_kicks_boot_when_no_thread_is_booting(tmp_path, monkeypatch): from hermes_cli.local_runtime.supervisor import state_path monkeypatch.setattr(ep, "_boot_in_flight", lambda config: True) - monkeypatch.setattr(ep, "_pid_alive", lambda pid: True) + monkeypatch.setattr( + "hermes_cli.local_runtime.recovery.legacy_recorded_process", lambda state: object()) monkeypatch.setattr("hermes_cli.local_runtime.detect.DEFAULT_PROBE_PORTS", ()) def _fake_ensure(config, force=False): @@ -657,13 +674,8 @@ def test_switch_model_explicit_llamacpp_provider(tmp_path, monkeypatch, stub_ser monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) from hermes_cli.local_runtime.supervisor import state_path - state_path().parent.mkdir(parents=True, exist_ok=True) - state_path().write_text(json.dumps({ - "base_url": f"http://127.0.0.1:{port}/v1", - # Live pid: ownership guard rejects health-200 + dead recorded pid - # (foreign server on our stable port). - "api_key": "sk-managed", "pid": os.getpid(), - }), encoding="utf-8") + _write_current_process_state( + state_path(), base_url=f"http://127.0.0.1:{port}/v1", api_key="sk-managed") from hermes_cli.model_switch import switch_model @@ -686,13 +698,8 @@ def test_runtime_provider_seam_llamacpp_alias(tmp_path, monkeypatch, stub_server port, handler = stub_server from hermes_cli.local_runtime.supervisor import state_path - state_path().parent.mkdir(parents=True, exist_ok=True) - state_path().write_text(json.dumps({ - # A LIVE pid: the ownership guard treats health-200 + dead recorded - # pid as a foreign server on our stable port (scratch-profile - # collision), so claiming this test process models "our server". - "base_url": f"http://127.0.0.1:{port}/v1", "api_key": "sk-managed", "pid": os.getpid(), - }), encoding="utf-8") + _write_current_process_state( + state_path(), base_url=f"http://127.0.0.1:{port}/v1", api_key="sk-managed") from hermes_cli.runtime_provider import _resolve_named_custom_runtime diff --git a/tests/hermes_cli/test_local_runtime_legacy_state.py b/tests/hermes_cli/test_local_runtime_legacy_state.py new file mode 100644 index 0000000000..8dfb2bc977 --- /dev/null +++ b/tests/hermes_cli/test_local_runtime_legacy_state.py @@ -0,0 +1,54 @@ +"""Legacy local-runtime records must not adopt a reused PID.""" +from __future__ import annotations + +import json +import os +from types import SimpleNamespace + +import pytest + + +@pytest.mark.parametrize(("case", "accepted"), [ + ("managed", True), + ("foreign-executable", False), + ("pid-reused", False), +]) +def test_legacy_endpoint_requires_managed_process_identity(tmp_path, monkeypatch, case, accepted): + from hermes_cli.local_runtime import bootstrap, endpoint, recovery, supervisor + + root = tmp_path / "runtimes" / "llamacpp" + root.mkdir(parents=True) + monkeypatch.setattr(supervisor, "runtimes_root", lambda: root) + models = tmp_path / "models" + monkeypatch.setattr(bootstrap, "models_dir", lambda: models) + + state = { + "pid": 647, + "base_url": "http://127.0.0.1:18434/v1", + "api_key": "legacy-test-key", + } + path = supervisor.state_path() + path.write_text(json.dumps(state), encoding="utf-8") + os.utime(path, (200, 200)) + + managed_exe = root / "b10964" / "metal" / "llama-server" + executable = tmp_path / "usr" / "sbin" / "distnoted" if case == "foreign-executable" else managed_exe + created = 201 if case == "pid-reused" else 100 + argv = [ + str(managed_exe), + "--host", "127.0.0.1", + "--port", "18434", + "--api-key", "legacy-test-key", + "--models-dir", str(models), + ] + process = SimpleNamespace( + exe=lambda: str(executable), + create_time=lambda: created, + cmdline=lambda: argv, + ) + monkeypatch.setattr(recovery.psutil, "Process", lambda pid: process) + monkeypatch.setattr(recovery.psutil, "pid_exists", lambda pid: True) + + expected = {"base_url": state["base_url"], "api_key": state["api_key"]} if accepted else None + assert recovery.legacy_recorded_process(state) is (process if accepted else None) + assert endpoint._state_endpoint() == expected