From 40087fba7de38e19297250d6bf86ef7eeca69104 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:56:46 +0530 Subject: [PATCH] fix(lifecycle-ledger): a --replace handover is no longer reported as an unclean death MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `detect_unclean_exit` decided "live owner mid-handover" by comparing the sentinel's `start_time` (the ledger claim, `time.time()` seconds) with `gateway.status.get_process_start_time(pid)` (proc clock ticks on Linux, centiseconds elsewhere — its own docstring says it is only comparable with itself). The two never matched, so every `--replace` takeover whose old owner was still tearing down read as a crash and was logged/persisted as one. The sentinel now carries the psutil `create_time` (stamped at claim since 8c3a35b69d), so the guard compares that with the live PID's create time — same producer, same unit. A pre-stamp sentinel cannot disambiguate PID reuse; a live PID is taken as the owner, as the psutil-silent case already was. Test drives all three cases; it fails on the previous ledger. Found while reviewing the start-attestation follow-ups (#110958). --- gateway/lifecycle_ledger.py | 26 +++++++++++++++++--------- tests/gateway/test_lifecycle_ledger.py | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/gateway/lifecycle_ledger.py b/gateway/lifecycle_ledger.py index 47f2bbbbbc..9c0732a7aa 100644 --- a/gateway/lifecycle_ledger.py +++ b/gateway/lifecycle_ledger.py @@ -104,23 +104,31 @@ def _append_exit_diag(record: Dict[str, Any], home: Optional[Path]) -> None: logger.debug("Failed to append unclean-exit record", exc_info=True) -def _pid_alive_with_start_time(pid: Any, start_time: Any) -> bool: - """True when ``pid`` is a live process matching ``start_time`` (±2s) — guards the - ``--replace`` race: a live matching owner mid-teardown is a handover, not a death.""" +def _pid_is_sentinel_owner(pid: Any, create_time: Any) -> bool: + """True when ``pid`` is a live process that is the sentinel's incarnation — guards the + ``--replace`` race: a live matching owner mid-teardown is a handover, not a death. + + Identity is the psutil ``create_time`` the sentinel stamps at claim (epoch seconds, same + producer on both sides). The sentinel's ``start_time`` is the ledger claim time and is NOT comparable with + ``gateway.status.get_process_start_time`` (proc ticks on Linux, centiseconds elsewhere) — the + old comparison never matched, so every ``--replace`` handover read as an unclean death. A + sentinel without ``create_time`` (pre-stamp gateway) cannot disambiguate PID reuse; a live PID + is taken as the owner, matching the psutil-silent case below.""" try: pid_int = int(pid) # NOT os.kill(pid, 0): on Windows that sends CTRL_C_EVENT to the target's console group. - from gateway.status import _pid_exists, get_process_start_time + from gateway.status import _pid_exists if pid_int <= 0 or not _pid_exists(pid_int): return False except Exception: return False - try: - actual = None if start_time is None else get_process_start_time(pid_int) - return actual is None or abs(float(actual) - float(start_time)) <= 2.0 # None: can't disambiguate PID reuse - except Exception: + if type(create_time) not in (int, float): return True + from hermes_cli.process_identity import _process_create_time + + actual = _process_create_time(pid_int) + return actual is None or abs(actual - float(create_time)) <= 2.0 # None: can't disambiguate PID reuse def _suspected_oom(mem: Dict[str, Any]) -> bool: @@ -141,7 +149,7 @@ def detect_unclean_exit(home: Optional[Path] = None) -> Optional[Dict[str, Any]] sentinel = _read_json(get_lifecycle_sentinel_path(home)) if not sentinel or sentinel.get("phase") != "running": return None - if _pid_alive_with_start_time(sentinel.get("pid"), sentinel.get("start_time")): + if _pid_is_sentinel_owner(sentinel.get("pid"), sentinel.get("create_time")): return None # live owner — planned takeover in flight, not a death evidence: Dict[str, Any] = { "prior_pid": sentinel.get("pid"), "prior_started_at": sentinel.get("started_at"), diff --git a/tests/gateway/test_lifecycle_ledger.py b/tests/gateway/test_lifecycle_ledger.py index d3e39a1497..4796d77be6 100644 --- a/tests/gateway/test_lifecycle_ledger.py +++ b/tests/gateway/test_lifecycle_ledger.py @@ -228,3 +228,25 @@ def test_sentinel_carries_process_birth_through_exit(tmp_path: Path, monkeypatch exited = json.loads(get_lifecycle_sentinel_path(tmp_path).read_text(encoding="utf-8")) assert exited["phase"] == "exited" assert (exited["start_time"], exited["create_time"]) == (running["start_time"], 1234.5) + + +def test_replace_handover_is_not_a_death_and_pid_reuse_is(tmp_path: Path, monkeypatch) -> None: + """The live-owner guard compares the sentinel's psutil ``create_time`` with the live PID's + (same producer, epoch seconds). The ledger's ``start_time`` (claim time) is never compared + with ``get_process_start_time`` (proc ticks / centiseconds): that comparison could not match, + so a ``--replace`` handover was reported as an unclean death.""" + from gateway import lifecycle_ledger + + monkeypatch.setattr(lifecycle_ledger, "_pid_exists", lambda pid: True, raising=False) + monkeypatch.setattr("gateway.status._pid_exists", lambda pid: True) + monkeypatch.setattr("hermes_cli.process_identity._process_create_time", lambda pid=None: 5000.0) + live = {"phase": "running", "pid": 4242, "start_time": 5003.7, "started_at": "x"} + + _write_sentinel(tmp_path, {**live, "create_time": 5000.0}) + assert detect_unclean_exit(home=tmp_path) is None # same incarnation still alive: handover + + _write_sentinel(tmp_path, {**live, "create_time": 4000.0}) + assert detect_unclean_exit(home=tmp_path) is not None # PID reused by another process: death + + _write_sentinel(tmp_path, live) # pre-stamp sentinel: a live PID is taken as the owner + assert detect_unclean_exit(home=tmp_path) is None