From 234fff615354aa5b823c2ebb7efd94e782aca113 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:43:56 +0530 Subject: [PATCH] fix(sessions): never reap an own lease younger than the vouch snapshot Follow-up on the #101437 salvage (#101415). `_own_live_lease_ids()` snapshots the server's session records under `_sessions_lock`, but the registry file lock is taken afterwards, and a sibling session attaches its lease to its record only after `try_acquire_active_session` has already written the entry. A finalize racing that acquire would read the brand-new lease as an orphan and drop it. Entries this process wrote inside a 30 s grace window are kept regardless of the snapshot; real orphans are minutes old. --- hermes_cli/active_sessions.py | 11 +++++ tests/hermes_cli/test_active_sessions.py | 43 +++++++++++++++++++ .../test_cross_process_orphan_ownership.py | 5 +++ 3 files changed, 59 insertions(+) diff --git a/hermes_cli/active_sessions.py b/hermes_cli/active_sessions.py index 4706909df4..20587125c0 100644 --- a/hermes_cli/active_sessions.py +++ b/hermes_cli/active_sessions.py @@ -757,6 +757,15 @@ def transfer_active_session( return updated +# A lease this process wrote in the last few seconds may not be in the +# caller's ``own_live_lease_ids`` yet: ``try_acquire_active_session`` writes +# the registry entry under the file lock and the server attaches the lease to +# its session record only after that returns. A concurrent finalize that +# snapshotted its live ids in between would otherwise read the brand-new lease +# as an orphan and drop it. Real orphans are minutes old (#101415). +_SELF_ORPHAN_GRACE_SECONDS = 30.0 + + def _drop_self_orphans( entries: list[dict[str, Any]], own_live_lease_ids: set[str] | None ) -> list[dict[str, Any]]: @@ -764,11 +773,13 @@ def _drop_self_orphans( if own_live_lease_ids is None: return entries pid = os.getpid() + cutoff = time.time() - _SELF_ORPHAN_GRACE_SECONDS return [ entry for entry in entries if entry.get("pid") != pid or str(entry.get("lease_id") or "") in own_live_lease_ids + or (_optional_float(entry.get("started_at")) or 0.0) > cutoff ] diff --git a/tests/hermes_cli/test_active_sessions.py b/tests/hermes_cli/test_active_sessions.py index e62ae64e78..0fe223101f 100644 --- a/tests/hermes_cli/test_active_sessions.py +++ b/tests/hermes_cli/test_active_sessions.py @@ -12,6 +12,17 @@ import pytest from hermes_cli import active_sessions + +def _backdate_leases(*homes, age_seconds=600.0): + """Age every lease in the given registries past the self-orphan grace.""" + for home in homes: + state_path = active_sessions._state_path(home) + entries = active_sessions._read_entries(state_path) + for entry in entries: + entry["started_at"] = time.time() - age_seconds + active_sessions._write_entries(state_path, entries) + + def test_resolve_max_concurrent_sessions_values(caplog): assert active_sessions.resolve_max_concurrent_sessions({}) is None assert active_sessions.resolve_max_concurrent_sessions({"max_concurrent_sessions": None}) is None @@ -164,6 +175,7 @@ def test_release_orphaned_leases_reclaims_only_unowned_own_pid_entries(tmp_path, + [{"lease_id": "elsewhere", "session_id": "other", "surface": "cli", "pid": os.getpid() }], ) + _backdate_leases(tmp_path / ".hermes") assert active_sessions.release_orphaned_leases({kept.lease_id, "elsewhere"}) == 1 assert sorted( entry["session_id"] @@ -192,6 +204,10 @@ def test_release_orphaned_leases_sweeps_profile_runtime_registries( assert root_lease is not None and root_error is None assert profile_lease is not None and profile_error is None + # A lease written seconds ago is never an orphan: a sibling finalize that + # snapshotted its live ids before this acquire must not reap it (#101415). + assert active_sessions.release_orphaned_leases(set()) == 0 + _backdate_leases(root, profile) assert active_sessions.release_orphaned_leases(set()) == 2 assert active_sessions.active_session_registry_snapshot(root) == [] assert active_sessions.active_session_registry_snapshot(profile) == [] @@ -603,3 +619,30 @@ def test_release_wins_against_transfer_waiting_on_same_lease_lock( assert lease.released is True assert active_sessions.active_session_registry_snapshot() == [] + + +def test_liveness_guard_keeps_a_just_acquired_own_lease_it_cannot_vouch_for( + tmp_path, monkeypatch +): + """Race in #101415's fix: the finalizing session snapshots its live lease + ids, then a sibling session acquires a lease before the registry lock is + taken. That lease is absent from the snapshot but is not an orphan.""" + home = tmp_path / ".hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + fresh, error = active_sessions.try_acquire_active_session( + session_id="fresh", surface="desktop", config={}, registry_home=home + ) + assert fresh is not None and error is None + + with active_sessions.active_session_liveness_guard( + "fresh", registry_home=home, own_live_lease_ids=set() + ) as active: + assert active is True + assert [e["lease_id"] for e in active_sessions.active_session_registry_snapshot(home)] == [fresh.lease_id] + + _backdate_leases(home) + with active_sessions.active_session_liveness_guard( + "fresh", registry_home=home, own_live_lease_ids=set() + ) as active: + assert active is False + assert active_sessions.active_session_registry_snapshot(home) == [] diff --git a/tests/tui_gateway/test_cross_process_orphan_ownership.py b/tests/tui_gateway/test_cross_process_orphan_ownership.py index c15d96512f..1a9de04ff9 100644 --- a/tests/tui_gateway/test_cross_process_orphan_ownership.py +++ b/tests/tui_gateway/test_cross_process_orphan_ownership.py @@ -305,6 +305,11 @@ def test_automatic_cleanup_reclaims_own_orphan_lease_not_treated_as_sibling( profile_home=profile_home, ) assert owner_lease is not None and message is None + # The owner vanished minutes ago; a lease written seconds ago is still + # inside the self-orphan grace window and must be left alone. + monkeypatch.setattr( + "hermes_cli.active_sessions._SELF_ORPHAN_GRACE_SECONDS", 0.0 + ) ended: list[tuple[str, str]] = [] class _FakeDB: