From 5c24458dffc23ddebf0c2caec25e9618d9cc7386 Mon Sep 17 00:00:00 2001 From: carlotestor Date: Tue, 22 Sep 2026 17:50:56 +0000 Subject: [PATCH] fix(sidebar): derive the route session cap from the shared limit The env override never reached /api/sessions because the route kept a separate hard-coded cap of 20. Resolve the window in api.config before profile init, clamp it at 200, and refuse a profile .env override. --- .env.example | 2 + README.md | 2 +- api/config.py | 15 +++++++ api/models.py | 6 +-- api/profiles.py | 7 +++- api/routes.py | 13 ++++-- tests/test_visible_session_limit_env.py | 54 +++++++++++++++++++++++-- 7 files changed, 86 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 2682308f..a96b87f5 100644 --- a/.env.example +++ b/.env.example @@ -135,6 +135,8 @@ # Sidebar recency window: how many recent sessions are listed (default: 20). # Also caps how many delegated subagent children can nest at once; raise it # for wide fan-outs. Invalid or non-positive values fall back to 20. +# Values above 200 are clamped. Resolved before profile init, so a profile +# .env cannot override this server-wide bound. # HERMES_WEBUI_VISIBLE_SESSION_LIMIT=20 # Directory scanned for WebUI plugins (default: ~/.hermes/plugins). diff --git a/README.md b/README.md index 3edf0f7b..76429aea 100644 --- a/README.md +++ b/README.md @@ -373,7 +373,7 @@ Full list of environment variables: | `HERMES_HOME` | Windows: `%LOCALAPPDATA%\hermes`; POSIX: `~/.hermes` | Base directory for Hermes state (affects all paths) | | `HERMES_CONFIG_PATH` | `$HERMES_HOME/config.yaml` | Path to Hermes config file | | `HERMES_WEBUI_SERVER_CWD` | *(unset)* | Working directory for the server process. Defaults to the agent dir; point it at a writable workspace when the agent dir is read-only so fallback relative writes land somewhere writable | -| `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` | `20` | Size of the sidebar's interactive recency window (how many recent non-cron/webhook sessions are listed). Also bounds how many delegated subagent children can nest at once, since a child only renders when its row wins a slot in the window — raise it for wide fan-outs. Non-integer or non-positive values fall back to the default | +| `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` | `20` | Size of the sidebar's interactive recency window (how many recent non-cron/webhook sessions are listed). Also bounds how many delegated subagent children can nest at once, since a child only renders when its row wins a slot in the window — raise it for wide fan-outs. Non-integer or non-positive values fall back to the default. Values above 200 are clamped. Resolved before profile init, so a profile `.env` cannot override it | | `HERMES_WEBUI_AGENT_CACHE_MAX` | `25` | Max live agent instances kept warm in the in-memory LRU. Each pins a full conversation transcript, so this is the dominant lever on resident memory — lower it on installs with many long sessions to cap RAM (at the cost of more cold reloads) | | `HERMES_WEBUI_SESSIONS_MAX` | `100` | Legacy operator override for the max compact `Session` objects held in the in-memory LRU. Prefer the `webui.sessions_cache_max` key in `config.yaml` (which takes precedence); this env var remains a fallback. Bounds resident memory so long-running installs cannot accumulate every session ever touched and eventually crash (#4765/#2233/#4633). Eviction only ever drops clean, persisted, non-active sessions; an evicted session lazily reloads from its JSON sidecar on next access | diff --git a/api/config.py b/api/config.py index 979ce3f7..dc933c86 100644 --- a/api/config.py +++ b/api/config.py @@ -68,6 +68,21 @@ def _env_int(name: str, default: int, *, minimum: int = 1) -> int: return default return value if value >= minimum else default + +def _env_int_clamped(name: str, default: int, *, minimum: int = 1, maximum: int) -> int: + """Like ``_env_int``, then clamp a valid override to ``maximum``.""" + value = _env_int(name, default, minimum=minimum) + if not str(os.getenv(name) or "").strip(): + return value + return min(value, maximum) + + +# Sidebar recency window. Resolved here, before profile init, so a profile +# .env cannot override a server-wide resource bound. Clamped at 200. +CLI_VISIBLE_SESSION_LIMIT = _env_int_clamped( + "HERMES_WEBUI_VISIBLE_SESSION_LIMIT", 20, maximum=200, +) + # ── TLS/HTTPS config (optional, env-overridable) ──────────────────────────── TLS_CERT = os.getenv("HERMES_WEBUI_TLS_CERT", "").strip() or None TLS_KEY = os.getenv("HERMES_WEBUI_TLS_KEY", "").strip() or None diff --git a/api/models.py b/api/models.py index 99226cb4..5330947d 100644 --- a/api/models.py +++ b/api/models.py @@ -50,9 +50,9 @@ from api.process_event_utils import stamp_message_source logger = logging.getLogger(__name__) # Size of the interactive sidebar recency window. Also bounds how many # delegated subagent children can be rendered at once, since a child only -# nests when it wins a slot in this window. Override with -# HERMES_WEBUI_VISIBLE_SESSION_LIMIT. -CLI_VISIBLE_SESSION_LIMIT = _cfg._env_int("HERMES_WEBUI_VISIBLE_SESSION_LIMIT", 20) +# nests when it wins a slot in this window. Resolved in api.config before +# profile init; override with HERMES_WEBUI_VISIBLE_SESSION_LIMIT (clamped 1–200). +CLI_VISIBLE_SESSION_LIMIT = _cfg.CLI_VISIBLE_SESSION_LIMIT # How many messageful cron sessions to surface in the project-chip layer. # Needs to exceed CLI_VISIBLE_SESSION_LIMIT so older cron runs stay # addressable even when many newer non-cron sessions dominate the default diff --git a/api/profiles.py b/api/profiles.py index 78f18735..58ff0143 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -198,7 +198,11 @@ def _unwrap_profile_home_to_base(home: Path) -> Path: # are operator/deployment-level postures, not per-profile toggles. Letting a # profile .env set HERMES_WEBUI_ISOLATED_PROFILE=0 would let a contained user # escape isolation (#4589). -_PROTECTED_ENV_KEYS = frozenset({'HERMES_WEBUI_ISOLATED_PROFILE'}) +_PROTECTED_ENV_KEYS = frozenset({ + 'HERMES_WEBUI_ISOLATED_PROFILE', + # Server-wide sidebar window; a profile .env must not widen the per-request query. + 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', +}) def _isolated_profile_opt_in() -> bool: @@ -934,6 +938,7 @@ _BLOCKED_RUNTIME_ENV_KEYS = { # #4589: operator/deployment isolation posture — never overridable by a # profile's own env on any runtime/gateway-parity path. 'HERMES_WEBUI_ISOLATED_PROFILE', + 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', } diff --git a/api/routes.py b/api/routes.py index 0e159784..3fd48b0e 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2521,8 +2521,8 @@ def _build_session_list_cache_payload( ) if show_cli_sessions: diag_stage("cli_cap") - archived_scoped = _cap_recent_cli_sessions(archived_scoped, cli_cap=CLI_VISIBLE_SESSION_CAP) - visible_scoped = _cap_recent_cli_sessions(visible_scoped, cli_cap=CLI_VISIBLE_SESSION_CAP) + archived_scoped = _cap_recent_cli_sessions(archived_scoped) + visible_scoped = _cap_recent_cli_sessions(visible_scoped) if visible_only: archived_scoped = [ s for s in archived_scoped if _session_has_server_visible_messages(s) @@ -10278,11 +10278,16 @@ def _dedupe_cli_sidebar_sessions_for_api( return _include_project_hidden_background_sidebar_sessions(candidates, visible) -CLI_VISIBLE_SESSION_CAP = 20 +def _cli_visible_session_cap() -> int: + """Shared sidebar window, not a second hard-coded 20.""" + from api.config import CLI_VISIBLE_SESSION_LIMIT + return CLI_VISIBLE_SESSION_LIMIT -def _cap_recent_cli_sessions(sessions: list[dict], cli_cap: int = CLI_VISIBLE_SESSION_CAP) -> list[dict]: +def _cap_recent_cli_sessions(sessions: list[dict], cli_cap: int | None = None) -> list[dict]: """Keep only the most recent CLI-visible sessions after filtering.""" + if cli_cap is None: + cli_cap = _cli_visible_session_cap() if cli_cap <= 0: return sessions kept = [] diff --git a/tests/test_visible_session_limit_env.py b/tests/test_visible_session_limit_env.py index e1fc0f32..e2e6e1ab 100644 --- a/tests/test_visible_session_limit_env.py +++ b/tests/test_visible_session_limit_env.py @@ -9,6 +9,10 @@ in a fresh subprocess (same pattern as test_issue3283_profiles_config_import_ord rather than ``importlib.reload``-ing it in the shared test process — reloading would recreate the module's locks, caches, and classes while other modules keep references to the old objects. + +The route cap must come from that same value. A profile ``.env`` must not +override it: the constant is resolved in ``api.config`` before profile init, +and the key is in ``api.profiles._PROTECTED_ENV_KEYS``. """ import os @@ -22,11 +26,28 @@ REPO_ROOT = Path(__file__).resolve().parents[1] _PROBE = """ import api.models +import api.routes print(api.models.CLI_VISIBLE_SESSION_LIMIT) +print(api.routes._cli_visible_session_cap()) +rows = [{"source": "cli", "title": "cli", "updated_at": i} for i in range(30)] +print(len(api.routes._cap_recent_cli_sessions(rows))) +""" + +_PROFILE_PROBE = """ +import os +from pathlib import Path +import api.config +import api.profiles +home = Path(os.environ["HERMES_HOME"]) +(home / ".env").write_text("HERMES_WEBUI_VISIBLE_SESSION_LIMIT=7\\n", encoding="utf-8") +api.profiles._reload_dotenv(home) +print("PROTECTED", "HERMES_WEBUI_VISIBLE_SESSION_LIMIT" in api.profiles._PROTECTED_ENV_KEYS) +print("ENV", os.environ.get("HERMES_WEBUI_VISIBLE_SESSION_LIMIT")) +print("LIMIT", api.config.CLI_VISIBLE_SESSION_LIMIT) """ -def _limit_in_fresh_interpreter(tmp_path, value): +def _run_probe(tmp_path, probe, value): home = tmp_path / "home" home.mkdir() env = os.environ.copy() @@ -34,21 +55,26 @@ def _limit_in_fresh_interpreter(tmp_path, value): if key.startswith("HERMES_WEBUI_") or key in ("HERMES_HOME", "HERMES_BASE_HOME"): env.pop(key) env["HOME"] = str(home) + env["HERMES_HOME"] = str(home) env["PYTHONPATH"] = str(REPO_ROOT) if value is not None: env["HERMES_WEBUI_VISIBLE_SESSION_LIMIT"] = value proc = subprocess.run( - [sys.executable, "-c", _PROBE], + [sys.executable, "-c", probe], cwd=REPO_ROOT, env=env, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - timeout=60, + timeout=90, check=False, ) assert proc.returncode == 0, proc.stderr - return int(proc.stdout.strip()) + return proc.stdout.strip().splitlines() + + +def _limit_in_fresh_interpreter(tmp_path, value): + return int(_run_probe(tmp_path, _PROBE, value)[0]) def test_defaults_to_20_when_unset(tmp_path): @@ -64,8 +90,28 @@ def test_invalid_or_nonpositive_falls_back_to_default(tmp_path, value): assert _limit_in_fresh_interpreter(tmp_path, value) == 20 +def test_values_above_200_are_clamped(tmp_path): + assert _limit_in_fresh_interpreter(tmp_path, "500") == 200 + + +def test_route_cap_follows_the_shared_value(tmp_path): + lines = _run_probe(tmp_path, _PROBE, "8") + assert lines[0] == "8" + assert lines[1] == "8" + assert lines[2] == "8" + + +def test_profile_env_cannot_override_visible_session_limit(tmp_path): + lines = _run_probe(tmp_path, _PROFILE_PROBE, "40") + assert lines[0] == "PROTECTED True" + assert lines[1] == "ENV 40" + assert lines[2] == "LIMIT 40" + + def test_setting_is_documented(): readme = (REPO_ROOT / "README.md").read_text(encoding="utf-8") env_example = (REPO_ROOT / ".env.example").read_text(encoding="utf-8") assert "| `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` | `20` |" in readme assert "# HERMES_WEBUI_VISIBLE_SESSION_LIMIT=20" in env_example + assert "200" in readme + assert "clamped" in env_example