From ca559a78523e9370bc2b46e689c5b7bb8ceb36a1 Mon Sep 17 00:00:00 2001 From: Rory Ford Date: Fri, 12 Jun 2026 21:04:41 +1000 Subject: [PATCH] fix(gateway): never prune sessions when active-process check fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prune_old_entries' active-process guard failed open: when has_active_processes_fn raised, the except block logged at debug and fell through to the age check, so sessions with live background processes attached could still be pruned — violating the documented invariant that such sessions are never dropped. Add a continue so an exception in the safety check fails safe (the entry is kept). Commit 6b408e131 fixed the session_key/session_id mismatch in this same guard but left the exception path failing open. Co-Authored-By: Claude Fable 5 --- gateway/session.py | 4 +++ tests/gateway/test_session_store_prune.py | 31 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/gateway/session.py b/gateway/session.py index f04090445e75..962d186c3513 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -2179,6 +2179,10 @@ class SessionStore: "has_active_processes_fn raised during prune for %s: %s", entry.session_key, exc, ) + # Fail safe: if we can't tell whether a background + # process is attached, keep the entry rather than + # risk orphaning live work. + continue if entry.updated_at < cutoff: removed_keys.append(key) for key in removed_keys: diff --git a/tests/gateway/test_session_store_prune.py b/tests/gateway/test_session_store_prune.py index b331d305e375..888d3a9a0a5d 100644 --- a/tests/gateway/test_session_store_prune.py +++ b/tests/gateway/test_session_store_prune.py @@ -165,6 +165,37 @@ class TestPruneBasics: assert removed == 1 assert "active" not in store._entries + def test_prune_keeps_entry_when_active_check_raises(self, tmp_path): + """A failing active-process check must fail safe, not fail open. + + If has_active_processes_fn raises, we can't tell whether a live + background process is attached — so the entry must be kept. + Previously the except block logged and fell through to the age + check, pruning the session anyway. + """ + def _broken(session_key: str) -> bool: + raise RuntimeError("process registry unavailable") + + store = _make_store(tmp_path, has_active_processes_fn=_broken) + store._entries["old"] = _entry("old", age_days=1000) + + removed = store.prune_old_entries(max_age_days=90) + + assert removed == 0 + assert "old" in store._entries + + def test_prune_removes_old_entry_when_active_check_returns_false(self, tmp_path): + """Sibling guard: a callback that cleanly reports no active process + must still allow the old entry to be pruned. + """ + store = _make_store(tmp_path, has_active_processes_fn=lambda key: False) + store._entries["old"] = _entry("old", age_days=1000) + + removed = store.prune_old_entries(max_age_days=90) + + assert removed == 1 + assert "old" not in store._entries + def test_prune_does_not_write_disk_when_no_removals(self, tmp_path): """If nothing is evictable, _save() should NOT be called.""" store = _make_store(tmp_path)