From 015171eba3371ddda96464dab5cced0f0f007b70 Mon Sep 17 00:00:00 2001 From: Ruby Hartono <58564005+rh-id@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:09:29 +0700 Subject: [PATCH] fix(models): cap _CLI_SESSIONS_CACHE with LRU eviction (drop-oldest) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Memory: the CLI/cron session projection cache was a plain dict with TTL storage but NO size cap and only lazy, same-key eviction. Each value is a copy.deepcopy() of the full CLI/cron session list (the expensive projection behind #4842/#4672). The cache key folds in a state.db content fingerprint that advances on every streamed message, so the next poll builds a NEW key; the previous key's heavy deepcopy was never looked up again, so its lazy-expiry path never ran. A long-lived process under churn could accumulate orphaned heavy deepcopies until the next structural clear_cli_sessions_cache(). Make it an OrderedDict capped at _CLI_SESSIONS_CACHE_MAX_ENTRIES (8) with drop-oldest on write (while len > MAX: popitem(last=False)) and move_to_end on fresh hits, mirroring the _CLAUDE_CODE_PARSE_CACHE / _SIDECAR_METADATA_CACHE LRU pattern in the same file. TTL remains the primary freshness control; the cap is the backstop the plain dict lacked. clear_cli_sessions_cache() keeps .clear(). Test: tests/test_cli_sessions_cache_cap.py — cache stays at len == MAX after MAX+5 distinct keys (oldest 5 evicted), a fresh hit marks the entry most-recently-used so it survives over colder entries, and the cache is an OrderedDict (the regression: it was an unbounded plain dict). Existing streaming-cache-TTL/cron-session tests unchanged. Contract Routing - Task type: memory-bound fix (unbounded projection cache). - Touched areas: CLI/cron sidebar projection cache (api/models.py _CLI_SESSIONS_CACHE). - Relevant public docs: AGENTS.md, CONTRIBUTING.md, docs/CONTRACTS.md, docs/rfcs/webui-run-state-consistency-contract.md (Sidebar/session metadata layer). - State layer mutated: Sidebar/session metadata layer — specifically the in-memory projection cache that backs the CLI/cron sidebar rows. The cache is a read-derivation of state.db content; it is NOT a source of truth. Dropped entries are rebuilt on the next poll (TTL/invalidation already drive rebuilds). - Invariant preserved — Invariant 4 (Maintenance is not activity): the cache is a derived projection; evicting an entry cannot move a session in the sidebar or change a row's content, because the next poll recomputes the projection from state.db. TTL remains the primary freshness control; the LRU cap only bounds how many STALE orphaned deepcopies linger between invalidations. Regression: test_cli_sessions_cache_cap.py plus the existing streaming-cache-TTL and cron-session tests (which exercise the real invalidate/rebuild path). - Scope boundaries: does not change sidebar ordering, unread state, updated_at, or the projection result. The cap is a memory backstop; correctness still flows from the existing TTL + clear-on-mutation invalidation. Release note: the CLI/cron sidebar projection cache is now LRU-bounded, capping worst-case memory for long-lived processes under heavy streaming churn. Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent). No AI-specific tool use mattered beyond the repo's standard read/edit/test workflow. --- api/models.py | 18 +++++- tests/test_cli_sessions_cache_cap.py | 89 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 tests/test_cli_sessions_cache_cap.py diff --git a/api/models.py b/api/models.py index 48a879a1..49888ad2 100644 --- a/api/models.py +++ b/api/models.py @@ -63,7 +63,16 @@ _CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 45.0 _CLI_SESSIONS_CACHE_LOCK = threading.Lock() _CLI_SESSIONS_CACHE_INFLIGHT: "dict[tuple, threading.Event]" = {} _CLI_SESSIONS_CACHE_INVALIDATION_VERSION = 0 -_CLI_SESSIONS_CACHE = {} +# LRU-bounded (drop-oldest) so a long-lived process under churn — where the +# state.db fingerprint advances on every streamed message and the structural +# clear-on-mutation listener doesn't fire for every fingerprint advance — can't +# accumulate orphaned heavy deepcopies. Each value is a copy.deepcopy() of the +# full CLI/cron session list (the expensive projection behind #4842/#4672), so +# the cap is deliberately small. TTL is still the primary freshness control; +# the cap is the backstop that the plain dict previously lacked. Mirrors the +# _CLAUDE_CODE_PARSE_CACHE / _SIDECAR_METADATA_CACHE LRU pattern. +_CLI_SESSIONS_CACHE: "collections.OrderedDict[tuple, tuple]" = collections.OrderedDict() +_CLI_SESSIONS_CACHE_MAX_ENTRIES = 8 _CLI_SESSIONS_CACHE_WAIT_SECONDS = 0.25 # Event waits that keep stale rows visible while a rebuild is in flight. _CLI_SESSIONS_CACHE_STALE_WAIT_SECONDS = 0.10 @@ -6405,6 +6414,9 @@ def _cache_cli_sessions_if_current( invalidation_stamp, _copy_cli_sessions(sessions), ) + _CLI_SESSIONS_CACHE.move_to_end(cache_key) + while len(_CLI_SESSIONS_CACHE) > _CLI_SESSIONS_CACHE_MAX_ENTRIES: + _CLI_SESSIONS_CACHE.popitem(last=False) return True @@ -6423,6 +6435,8 @@ def _copy_fresh_cli_sessions_cache_entry(cache_key: tuple): return None if cached_expires_at <= time.monotonic(): return None + # LRU: a fresh hit is the most-recently-used entry. + _CLI_SESSIONS_CACHE.move_to_end(cache_key) return _copy_cli_sessions(cached_sessions) @@ -7272,6 +7286,8 @@ def get_cli_sessions( if cached_stamp != _CLI_SESSIONS_CACHE_INVALIDATION_VERSION: _CLI_SESSIONS_CACHE.pop(cache_key, None) elif cached_expires_at > now: + # LRU: a fresh hit is the most-recently-used entry. + _CLI_SESSIONS_CACHE.move_to_end(cache_key) return _copy_cli_sessions(cached_sessions) else: stale_sessions = _copy_cli_sessions(cached_sessions) diff --git a/tests/test_cli_sessions_cache_cap.py b/tests/test_cli_sessions_cache_cap.py new file mode 100644 index 00000000..7d2af868 --- /dev/null +++ b/tests/test_cli_sessions_cache_cap.py @@ -0,0 +1,89 @@ +"""Tests: ``_CLI_SESSIONS_CACHE`` is LRU-bounded (drop-oldest). + +Regression: the CLI/cron session projection cache was a plain ``dict`` with TTL +storage but NO size cap and only lazy, same-key eviction. Each value is a +``copy.deepcopy()`` of the full CLI/cron session list (the expensive projection +behind #4842/#4672). The cache key folds in a state.db content fingerprint that +advances on every streamed message, so the next poll builds a NEW key; the +previous key's heavy deepcopy was never looked up again, so its lazy-expiry path +never ran. A long-lived process under churn could accumulate orphaned heavy +deepcopies until the next structural ``clear_cli_sessions_cache()``. + +The cache is now an ``OrderedDict`` capped at ``_CLI_SESSIONS_CACHE_MAX_ENTRIES`` +with drop-oldest on write (mirroring ``_CLAUDE_CODE_PARSE_CACHE``) and +``move_to_end`` on fresh hits. TTL remains the primary freshness control; the +cap is the backstop the plain dict lacked. +""" +from __future__ import annotations + +import api.models as models +from api.models import ( + _CLI_SESSIONS_CACHE_MAX_ENTRIES, + _cache_cli_sessions_if_current, + _cli_sessions_cache_invalidation_stamp, + _copy_fresh_cli_sessions_cache_entry, +) + + +def _reset_cache(): + with models._CLI_SESSIONS_CACHE_LOCK: + models._CLI_SESSIONS_CACHE.clear() + + +def test_cache_is_bounded_drops_oldest_on_write(): + """Inserting more than MAX_ENTRIES distinct keys keeps len == MAX and evicts + the OLDEST (least-recently-used) entry.""" + _reset_cache() + stamp = _cli_sessions_cache_invalidation_stamp() + cap = _CLI_SESSIONS_CACHE_MAX_ENTRIES + assert cap >= 1 + + # Insert cap + 5 distinct keys, each a distinct payload so we can track them. + for i in range(cap + 5): + key = (f"profile-{i}",) + ok = _cache_cli_sessions_if_current( + key, ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": i}] + ) + assert ok + + with models._CLI_SESSIONS_CACHE_LOCK: + assert len(models._CLI_SESSIONS_CACHE) == cap + keys = list(models._CLI_SESSIONS_CACHE.keys()) + # The oldest 5 (profile-0..4) were evicted; the newest `cap` remain. + assert keys[0] == ("profile-5",) + assert keys[-1] == (f"profile-{cap + 5 - 1}",) + assert ("profile-0",) not in keys + + +def test_fresh_hit_marks_entry_most_recently_used(): + """A fresh cache hit moves the entry to the end (most-recently-used), so a + frequently-read entry is NOT evicted over colder ones.""" + _reset_cache() + stamp = _cli_sessions_cache_invalidation_stamp() + cap = _CLI_SESSIONS_CACHE_MAX_ENTRIES + + # Fill the cache exactly. + for i in range(cap): + _cache_cli_sessions_if_current( + (f"k{i}",), ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": i}] + ) + # Touch k0 (the oldest) so it becomes most-recently-used. + served = _copy_fresh_cli_sessions_cache_entry(("k0",)) + assert served == [{"id": 0}] + # Now insert one more; the eviction victim should be k1, NOT k0. + _cache_cli_sessions_if_current( + ("k_new",), ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": 999}] + ) + with models._CLI_SESSIONS_CACHE_LOCK: + keys = list(models._CLI_SESSIONS_CACHE.keys()) + assert ("k0",) in keys # survived because it was just used + assert ("k1",) not in keys # evicted as the new oldest + assert ("k_new",) in keys + + +def test_cache_is_ordered_dict_not_plain_dict(): + """The regression: the cache used to be a plain dict with no ordering. It + must now be an OrderedDict to support move_to_end / popitem(last=False).""" + import collections + + assert isinstance(models._CLI_SESSIONS_CACHE, collections.OrderedDict)