From c9dbdbcea735dae65e7c9fc6c9d4a6983ff156be Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 17 Aug 2026 23:06:45 +0000 Subject: [PATCH] fix(gateway): grace window keeps first-call goal persistence on healthy DBs Review finding on the off-loop bootstrap: returning None on every cold- cache loop-thread call silently dropped the first goal/heartbeat persistence op even when the DB was perfectly healthy. The loop-thread path now waits up to 250ms on the bootstrap event - a healthy init (tens of ms) completes inside the window and the caller gets the real DB; a contended init (the crash-loop scenario) exceeds it and degrades to None with a bounded, watchdog-safe stall. --- hermes_cli/goals.py | 30 ++++++++++++++----- .../test_goals_db_bootstrap_off_loop.py | 25 +++++++++------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/hermes_cli/goals.py b/hermes_cli/goals.py index 7839e817db..db00cabb3b 100644 --- a/hermes_cli/goals.py +++ b/hermes_cli/goals.py @@ -664,10 +664,17 @@ def _meta_key(session_id: str) -> str: _DB_CACHE: Dict[str, Any] = {} _DB_BOOTSTRAP_LOCK = threading.Lock() -_DB_BOOTSTRAP_INFLIGHT: set = set() +_DB_BOOTSTRAP_INFLIGHT: Dict[str, threading.Event] = {} + +# How long a loop-thread caller waits for the background bootstrap before +# degrading to None. Normal SessionDB init is ~10-100ms, so the common case +# still returns a real DB (no silently dropped goal writes); a contended +# init (locked state.db mid-migration) blows past this and the caller +# degrades, with the loop stalled far under the watchdog's probe window. +_DB_BOOTSTRAP_LOOP_WAIT_S = 0.25 -def _bootstrap_session_db(home: str) -> None: +def _bootstrap_session_db(home: str, done: threading.Event) -> None: """Construct SessionDB off-loop and populate the cache (worker thread).""" try: from hermes_state import SessionDB @@ -679,7 +686,8 @@ def _bootstrap_session_db(home: str) -> None: with _DB_BOOTSTRAP_LOCK: if db is not None and home not in _DB_CACHE: _DB_CACHE[home] = db - _DB_BOOTSTRAP_INFLIGHT.discard(home) + _DB_BOOTSTRAP_INFLIGHT.pop(home, None) + done.set() def _get_session_db() -> Optional[Any]: @@ -727,15 +735,23 @@ def _get_session_db() -> Optional[Any]: cached = _DB_CACHE.get(home) if cached is not None: return cached - if home not in _DB_BOOTSTRAP_INFLIGHT: - _DB_BOOTSTRAP_INFLIGHT.add(home) + done = _DB_BOOTSTRAP_INFLIGHT.get(home) + if done is None: + done = threading.Event() + _DB_BOOTSTRAP_INFLIGHT[home] = done threading.Thread( target=_bootstrap_session_db, - args=(home,), + args=(home, done), name="goals-sessiondb-bootstrap", daemon=True, ).start() - return None + # Grace window: a healthy init finishes in tens of ms, so waiting + # briefly keeps goal/heartbeat persistence working on the very first + # loop-thread call instead of silently dropping it. A contended init + # (the crash-loop scenario) exceeds the window and we degrade to + # None — a bounded stall far below the watchdog's probe timeout. + done.wait(_DB_BOOTSTRAP_LOOP_WAIT_S) + return _DB_CACHE.get(home) try: db = SessionDB() diff --git a/tests/hermes_cli/test_goals_db_bootstrap_off_loop.py b/tests/hermes_cli/test_goals_db_bootstrap_off_loop.py index 12adc5b279..011d4accfa 100644 --- a/tests/hermes_cli/test_goals_db_bootstrap_off_loop.py +++ b/tests/hermes_cli/test_goals_db_bootstrap_off_loop.py @@ -53,9 +53,10 @@ def _patch_sessiondb(monkeypatch): monkeypatch.setattr(hermes_state, "SessionDB", _RecordingDB) -def test_loop_thread_cache_miss_returns_none_without_constructing(monkeypatch): - """On the event-loop thread with a cold cache: no inline construction, - immediate None, background population.""" +def test_loop_thread_cache_miss_constructs_off_loop(monkeypatch): + """On the event-loop thread with a cold cache: construction happens on a + background thread (never the loop thread). A fast init is returned via + the grace window; either way the loop thread never runs SessionDB().""" _patch_sessiondb(monkeypatch) loop_thread_id = None inline_result = "UNSET" @@ -67,9 +68,10 @@ def test_loop_thread_cache_miss_returns_none_without_constructing(monkeypatch): asyncio.run(main()) - # The loop-thread call must not have blocked on construction. - assert inline_result is None - # Background thread eventually populates the cache, OFF the loop thread. + # Fast init: the grace window returns the real instance (no silent + # feature loss on the first call). + assert isinstance(inline_result, _RecordingDB) + # Cache populated for subsequent calls. deadline = time.monotonic() + 5 while time.monotonic() < deadline and not goals._DB_CACHE: time.sleep(0.02) @@ -108,7 +110,8 @@ def test_worker_thread_constructs_inline(monkeypatch): def test_slow_construction_does_not_block_the_loop(monkeypatch): """A SessionDB whose init blocks (locked-DB migration) must not stall - the event loop past the watchdog probe window.""" + the event loop past the watchdog probe window: bounded grace wait, + then degrade to None.""" import hermes_state class _BlockingDB: @@ -120,14 +123,16 @@ def test_slow_construction_does_not_block_the_loop(monkeypatch): monkeypatch.setattr(hermes_state, "SessionDB", _BlockingDB) elapsed = None + result = "UNSET" async def main(): - nonlocal elapsed + nonlocal elapsed, result t0 = time.monotonic() - goals._get_session_db() + result = goals._get_session_db() elapsed = time.monotonic() - t0 asyncio.run(main()) - assert elapsed is not None and elapsed < 0.5, ( + assert result is None, "contended init must degrade to None" + assert elapsed is not None and elapsed < 1.0, ( f"loop-thread call blocked for {elapsed:.2f}s — watchdog territory" )